Skip to content

Conversation

@clue
Copy link
Member

@clue clue commented Jun 16, 2019

This changeset is in preparation for upcoming refactorings to move
unrelated logic out of the parser class to prepare for persistent HTTP
connections in follow-up PR. This changeset does not affect the public
API and happens to improves performance slightly from around 9000 req/s
to 9200 req/s on my machine (best of 5).

We could expose the internal RequestHeaderParser as input to the StreamingServer in order to solve #214, but I've decided against this change for now: Doing so would mean that we expose an otherwise @internal class to the outside and thus make future changes to this class more difficult. The current changeset can be used as a base for future changes in this direction or some other option array as discussed in #214, but I'd rather leave this up for a future discussion.

Builds on top of #345
Refs #39
Closes #241
Closes #242

@clue clue added this to the v0.8.5 milestone Jun 16, 2019
@WyriHaximus WyriHaximus requested review from WyriHaximus and jsor June 16, 2019 15:06
@WyriHaximus
Copy link
Member

@clue unit tests are failing 😝

This changeset is in preparation for upcoming refactorings to move
unrelated logic out of the parser class to prepare for persistent HTTP
connections in follow-up PR. This changeset does not affect the public
API and happens to improves performance slightly from around 9000 req/s
to 9200 req/s on my machine (best of 5).
@clue
Copy link
Member Author

clue commented Jun 16, 2019

@WyriHaximus Updated to fix failing tests on legacy PHP :shipit:

@WyriHaximus
Copy link
Member

@clue 👍 , will review tonight it's BBQ time!

@jsor jsor merged commit f30fb12 into reactphp:master Jun 17, 2019
@clue clue deleted the reuse-parser branch June 17, 2019 16:16
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.

3 participants