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 of Loop by avoiding unneeded method calls #266

Merged
merged 1 commit into from
May 5, 2023

Conversation

clue
Copy link
Member

@clue clue commented May 4, 2023

This changeset improves performance of Loop slightly by avoiding unneeded method calls. Not the most significant improvement, but given the small changeset and how we plan to make the default Loop more prominent in ReactPHP v3, I figured this change makes sense in either case:

$ time php examples/91-benchmark-ticks.php 1000000
// old 0.83
// new 0.76

Builds on top of #245 and #229
See also https://github.com/orgs/reactphp/discussions/481

@clue clue added this to the v1.4.0 milestone May 4, 2023
@clue clue force-pushed the perf branch 3 times, most recently from 3d28bd1 to f292a30 Compare May 4, 2023 14:28
@clue
Copy link
Member Author

clue commented May 5, 2023

Updated to work around all legacy PHP and Windows quirks, the result is a somewhat less compact changeset, but still shows the same performance improvements. One more reason to drop legacy support in upcoming ReactPHP v3 (https://github.com/orgs/reactphp/discussions/481) to simplify this significantly! This is ready for review. :shipit:

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.

Not as pretty as we discussed earlier this week, but a good step to improve performance for this 👍

@SimonFrings SimonFrings merged commit 1e7460b into reactphp:1.x May 5, 2023
29 checks passed
@SimonFrings SimonFrings deleted the perf branch May 5, 2023 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants