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

Correct the http benchmarks #139

Merged
merged 2 commits into from
Apr 19, 2018

Conversation

andrewthad
Copy link
Contributor

@andrewthad andrewthad commented Mar 19, 2018

There two two unrelated issues with them. The first was that the samples had extra newline breaks in them which would cause the request and the response to be parsed incorrectly. The second was that the warp request parser was used incorrectly. It needs to be fed a list of bytestring that has already been split on newlines. The results of the changes are that the request parsers for both warp and attoparsec are significantly slower. The warp one is about 15x slower than it previously was because it previously did almost no work. It is not totally clear why the attoparsec one is slower, but it is 3x slower than it previously was.

with them. The first was that the samples had extra newline
breaks in them which would cause the request and the response
to be parsed incorrectly. The second was that the warp
request parser was used incorrectly. It needs to be fed a
list of bytestring that has already been split on newlines.
The results of the changes are that the request parsers for
both warp and attoparsec are significantly slower. The
warp one is about 15x slower than it previously was because
it previously did almost no work. It is not totally clear
why the attoparsec one is slower, but it is 3x slower than
it previously was.
@andrewthad
Copy link
Contributor Author

I just realized why the parsing code is three times slower. It’s because the parser was previously failing when it tried to parse a header that had been line wrapped.

@andrewthad
Copy link
Contributor Author

Also, it’s a little strange that the parser uses inClass, which is specifically discouraged in the documentation. I suspect that it also contributes to the slowness. I am going to change that as well.

For what it’s worth, the warp http header parser does not appear to do any validation of the header name. This difference is probably worth noting in the benchmarking code. It makes it nearly impossible for attoparsec to match the performance of warp in this case.

…ossibility of matching the performance of warp.
@andrewthad
Copy link
Contributor Author

I've added a note about warp. Also, I've removed the use of inClass.

@bgamari
Copy link
Collaborator

bgamari commented Apr 17, 2018

Thanks! I've opened bgamari#1 to test under CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants