Skip to content

Drop child process adapter #108

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

Merged

Conversation

WyriHaximus
Copy link
Member

While fully non-blocking the child process adapter is a factor 100 slower than all the other adapters combined, including the blocking fallback adapter. As such removing it wil increase performance for everyone not having ext-uv or ext-eio (coming in a follow-up PR) installed. Even at the cost of blocking for a short period of time.

@WyriHaximus WyriHaximus added this to the v0.2.0 milestone Aug 24, 2022
@WyriHaximus WyriHaximus requested a review from clue August 24, 2022 14:18
@clue clue mentioned this pull request Aug 24, 2022
@WyriHaximus WyriHaximus marked this pull request as ready for review August 24, 2022 15:41
While fully non-blocking the child process adapter is a factor 100 slower than all the other adapters combined, including the blocking fallback adapter. As such removing it wil increase performance for everyone not having ext-uv or ext-eio (coming in a follow-up PR) installed. Even at the cost of blocking for a short period of time.
@WyriHaximus WyriHaximus force-pushed the 0.2.x-drop-child-process-adapter branch from 6e50c5d to 3933029 Compare August 25, 2022 07:45
@clue
Copy link
Member

clue commented Aug 26, 2022

@WyriHaximus I agree that it might be useful to remove the child process adapter for now given the major refactoring (#97 and friends). I still think this might be useful again in the future, but I'm happy to remove any excess code for the time being 👍

(Not sure about the PR labels, I'll add feature and BC break, but feel free to update as you see fit.)

@clue clue merged commit 57d8f3a into reactphp:0.2.x Aug 26, 2022
@WyriHaximus WyriHaximus deleted the 0.2.x-drop-child-process-adapter branch August 26, 2022 17:02
@WyriHaximus
Copy link
Member Author

@clue When the time comes, and we can get it to run it's test in under a second we should consider adding it. But a 100 fold in performance difference is just to big.

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.

2 participants