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

Performance degradation #329

Closed
slimmy opened this issue Jun 3, 2015 · 12 comments
Closed

Performance degradation #329

slimmy opened this issue Jun 3, 2015 · 12 comments

Comments

@slimmy
Copy link

@slimmy slimmy commented Jun 3, 2015

This issue is in response to issue 88 for the micro framework echo.

First of all echo is about 8x faster than Gin in that benchmark. Second, Gin went from 69373 ns/op to 412886 ns/op, nearly 6 times slower, what happened?

Since performance is one of the main selling points for Gin I think this is worth discussing. I really like Gin but the reason I started using it was because of the performance, I assume this is the case for many others too.

Here are the relevant numbers

April 1, 2015
BenchmarkEcho_GithubAll 30000 42728 ns/op 0 B/op 0 allocs/op
BenchmarkGin_GithubAll 20000 69373 ns/op 13792 B/op 167 allocs/op
June 3, 2015
BenchmarkEcho_GithubAll 30000 51470 ns/op 0 B/op 0 allocs/op
BenchmarkGin_GithubAll 3000 412886 ns/op 0 B/op 0 allocs/op
@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 3, 2015

yes, it worths discussing. I am investigating. Something must be wrong.

@slimmy
Copy link
Author

@slimmy slimmy commented Jun 3, 2015

Yeah obviously Gin is more of a full fledged framework than echo but maybe there are some ideas that can be borrowed. Also the drastic slowdown is definitely worth investigating.

@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 3, 2015

Right now echo, should be only 10% faster.
BenchmarkEcho_GithubAll 30000 51470 ns/op 0 B/op 0 allocs/op
BenchmarkGin_GithubAll 3000 412886 ns/op 0 B/op 0 allocs/op

is not acceptable.

@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 3, 2015

@slimmy performance regression fixed in my local copy

BenchmarkGin_GithubAll     30000         52138 ns/op           0 B/op          0 allocs/op
BenchmarkHttpRouter_GithubAll      20000         62015 ns/op       13792 B/op        167 allocs/op

@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 3, 2015

Gin was creating a different tree! for each route!!!

@slimmy
Copy link
Author

@slimmy slimmy commented Jun 3, 2015

Well then, should be an easy fix and put the performance right back on track. Good job finding it!

@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 3, 2015

0b043d0

what a incredible bug! when I saw it I could not believe the unit tests where passing. In fact it didn't break anything.

@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 4, 2015

@slimmy echo faster?

BenchmarkEcho_Param 10000000           129 ns/op           0 B/op          0 allocs/op
BenchmarkGin_Param  10000000           130 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_Param5     5000000           258 ns/op           0 B/op          0 allocs/op
BenchmarkGin_Param5  5000000           221 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_Param20    2000000           690 ns/op           0 B/op          0 allocs/op
BenchmarkGin_Param20     3000000           542 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_ParamWrite     5000000           243 ns/op           8 B/op          1 allocs/op
BenchmarkGin_ParamWrite  5000000           324 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_GithubStatic  10000000           170 ns/op           0 B/op          0 allocs/op
BenchmarkGin_GithubStatic   10000000           157 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_GithubParam    5000000           296 ns/op           0 B/op          0 allocs/op
BenchmarkGin_GithubParam     5000000           250 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_GithubAll    30000         59623 ns/op           0 B/op          0 allocs/op
BenchmarkGin_GithubAll     30000         51971 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_GPlusStatic   10000000           127 ns/op           0 B/op          0 allocs/op
BenchmarkGin_GPlusStatic    20000000           123 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_GPlusParam    10000000           173 ns/op           0 B/op          0 allocs/op
BenchmarkGin_GPlusParam 10000000           168 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_GPlus2Params   5000000           238 ns/op           0 B/op          0 allocs/op
BenchmarkGin_GPlus2Params   10000000           209 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_GPlusAll    500000          2934 ns/op           0 B/op          0 allocs/op
BenchmarkGin_GPlusAll     500000          2564 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_ParseStatic   10000000           131 ns/op           0 B/op          0 allocs/op
BenchmarkGin_ParseStatic    10000000           128 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_ParseParam    10000000           143 ns/op           0 B/op          0 allocs/op
BenchmarkGin_ParseParam 10000000           138 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_Parse2Params  10000000           181 ns/op           0 B/op          0 allocs/op
BenchmarkGin_Parse2Params   10000000           164 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_ParseAll    300000          5227 ns/op           0 B/op          0 allocs/op
BenchmarkGin_ParseAll     300000          4734 ns/op           0 B/op          0 allocs/op
BenchmarkEcho_StaticAll    50000         40169 ns/op           0 B/op          0 allocs/op
BenchmarkGin_StaticAll     50000         33080 ns/op           0 B/op          0 allocs/op

only one test: BenchmarkEcho_ParamWrite
this is because Gin has an addition indirection, it allows gzip, and caching middlewares

@slimmy
Copy link
Author

@slimmy slimmy commented Jun 4, 2015

Nice, all is back to normal :)

Understandable that the unit tests didn't catch the lazy creation of the root in that case. Maybe it would be wise to run the benchmarks on a more regular interval to catch things like these.

@slimmy slimmy closed this Jun 4, 2015
@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 4, 2015

@slimmy yes! in fact I am working in my own benchmarking tests. I already wrote some. They tests everything, JSON parsing, rendering, 404 handling... several middlewares

@slimmy
Copy link
Author

@slimmy slimmy commented Jun 4, 2015

@manucorporat Sounds promising! It would be cool to have a service that automatically would run benchmarks on each git push and report anomalies etc. Maybe I'll look into that in the future if I get some time over.

@manucorporat
Copy link
Contributor

@manucorporat manucorporat commented Jun 4, 2015

@slimmy
the benchmarks are already helping to identify places where performance can be improved:

822b995
70325de
56683d3
835f66f

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

Successfully merging a pull request may close this issue.

None yet
2 participants