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

fix path pattern for aero in Param20 benchmark #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PratikDeoghare
Copy link
Contributor

@PratikDeoghare PratikDeoghare commented Jul 26, 2020

BenchmarkAero_Param5       	17845056	        66.8 ns/op	       0 B/op	       0 allocs/op
BenchmarkAero_Param20      	40719792	        29.2 ns/op	       0 B/op	       0 allocs/op

BenchmarkAero_Param20 took less time than BenchmarkAero_Param5 which made me
suspicious. BenchmarkAero_Param20 is using twentyBrace = /{a}/{b}/... so aero treats the
path as static. This fixes the benchmark to use the pattern twentyColon = /:a/:b/....

That changes the above benchmarks to

BenchmarkAero_Param5       	17763198	        66.5 ns/op	       0 B/op	       0 allocs/op
BenchmarkAero_Param20      	 6814093	       178 ns/op	       0 B/op	       0 allocs/op

@PratikDeoghare
Copy link
Contributor Author

PratikDeoghare commented Jul 26, 2020

Ah wait! Aero supports 16 params max. I had locally edited branch of Aero.

If this PR merges BenchmarkAero_Param20 will panic.

EDIT: Created an issue aerogo/aero#19

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.

1 participant