-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix falcore benchmark #2270
Fix falcore benchmark #2270
Conversation
@methane thanks for the quick turnaround on the fix! Out of curiosity, how did removing the hardcoded content types fix the issue? |
http.Header is typedef of map. Some handlers returns same map instance. Then, following filter adds headers to the http.Header which http handlers returns.
|
Ahh, makes sense. Yeah, I figured there was something that was returning the same instance across threads. Did some test runs locally and it seems to be running okay for me! the only thing I noticed was that, when I run the plaintext test with the pipeline.lua script like it is in the suite:
I get repeated output that looks like this:
Plaintext is still returning with responses, but perhaps its performance is less than it should be. In cases where I get that error, wrk is showing very high latency. I think this might be a result of the pipeline.lua script, which, as I understand it, generates ~16x more requests. Generally, max latency increases the more connections that are added, so the pipeline.lua script may just be exacerbating the problem. For comparison, these are my latency results with and without the lua script: without pipeline.lua
with pipeline.lua
Anyway, those are just my observations in my local testing. If you're satisfied with the falcore results, I'm happy to merge this in! |
The error may happens while wrk is stopping. (wrk may not wait all response of pipelined request.) If falcore result is very worse than other Go frameworks, I'll investigate. Go ahead. |
fixes #2269