Skip to content

Conversation

@clue
Copy link
Member

@clueclue 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

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

@WyriHaximusWyriHaximus 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

@SimonFringsSimonFrings left a comment

Choose a reason for hiding this comment

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

Nice 👍

@SimonFringsSimonFrings merged commit 9c3dc0e into reactphp:4.xMay 24, 2024
@SimonFringsSimonFrings deleted the cleanup-map branch May 24, 2024 07:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@clue@WyriHaximus@SimonFrings