Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance by avoiding unneeded references in FiberMap #88

Merged
merged 1 commit into from
May 24, 2024

Conversation

clue
Copy link
Member

@clue clue commented May 23, 2024

This change applies some performance improvements by avoiding unneeded references in the internal FiberMap. This is a purely internal change that comes with 100% code coverage and does not otherwise affect the public API, so it should be safe to apply.

This happens to show a ~10% performance improvement in my synthetic benchmark:

async(function () {
    $promise = resolve(42);
    $n = 10_000_000;

    for ($i = 0; $i < $n; ++$i) {
        await($promise);
    }
})();

// old: 3.9s
// new: 3.5s

(Also posted in https://twitter.com/another_clue/status/1793723464194638108)

This code path only affects v4, so there is no need to backport this to v3 or v2.

Builds on top of #55 and #20

@clue clue added the new feature New feature or request label May 23, 2024
@clue clue added this to the v4.3.0 milestone May 23, 2024
@clue clue marked this pull request as ready for review May 23, 2024 19:27
Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! Lets get this shipped

Copy link
Member

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

@SimonFrings SimonFrings merged commit 9c3dc0e into reactphp:4.x May 24, 2024
6 checks passed
@SimonFrings SimonFrings deleted the cleanup-map branch May 24, 2024 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants