Change unnecessary string comparison to length check in Go benchmarks#29
Open
tuommaki wants to merge 1 commit intosqueaky-pl:masterfrom
Open
Conversation
Because router already guarantees that URL starts with '/', it's unnecessary to compare the path string further for 404 check. One may as well check that it's length matches expected one character. Doesn't make huge difference on Go standard library version, but gives couple thousand req/s more for fasthttp version. Also made 404 response writing a cleaner, while at there.
|
Any update ? @squeaky-pl |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because router already guarantees that URL starts with '/', it's
unnecessary to compare the path string further for 404 check. One may as
well check that it's length matches expected one character.
Doesn't make huge difference on Go standard library version, but gives
couple thousand req/s more for fasthttp version.
Also made 404 response writing a cleaner, while at there.
I run my tests on idle Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz and got some interesting results. First, japronto benchmark results were about in line with your AWS results, but Go versions were noticeably faster - probably due to faster processor clock speed. I tested with Go 1.7.4 and 1.8.0rc3. From these, the stable version was noticeably faster for stdlib performance, but for fasthttp it was other way round. What's behind this, is not yet clear. Numbers are in the end. All tests were run with:
wrk -t 1 -c 100 -d 2m -s misc/pipeline.lua http://localhost:8080
With latest Go 1.8.0rc3 and my change, the golang-fasthttp version was ~1.195x faster than japronto.
japronto:
Requests/sec: 1249535.78
Transfer/sec: 109.63MB
Go before change:
Go 1.7.4:
stdlib:
Requests/sec: 100442.66
Transfer/sec: 12.36MB
fasthttp:
Requests/sec: 1429564.72
Transfer/sec: 200.41MB
Go 1.8.0rc3:
stdlib:
Requests/sec: 88058.14
Transfer/sec: 10.83MB
fasthttp:
Requests/sec: 1477820.61
Transfer/sec: 207.18MB
After this change:
Go 1.7.4:
stdlib:
Requests/sec: 101756.51
Transfer/sec: 12.52MB
fasthttp:
Requests/sec: 1468705.08
Transfer/sec: 205.90MB
Go 1.8.0rc3:
stdlib:
Requests/sec: 87261.03
Transfer/sec: 10.74MB
fasthttp:
Requests/sec: 1494392.48
Transfer/sec: 209.50MB