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

Strings are not the fastpath #9

Merged
merged 1 commit into from
Apr 5, 2016

Conversation

sdboyer
Copy link
Member

@sdboyer sdboyer commented Mar 29, 2016

This pulls over a commit from #7 , because strings are not the fastpath. This one change makes a crazy amount of difference:

notfastpath

With absolute numbers, that's:

benchmark                                  old ns/op     new ns/op     delta
BenchmarkCheckVersionRange-8               1987          22.1          -98.89%
BenchmarkCheckVersionUnion-8               2047          31.2          -98.48%
BenchmarkValidateVersionRange-8            1998          30.7          -98.46%
BenchmarkCheckVersionCaret-8               1000          17.7          -98.23%
BenchmarkCheckVersionTilde-8               1032          19.1          -98.15%
BenchmarkCheckVersionUnary-8               1012          19.8          -98.04%
BenchmarkValidateVersionCaret-8            1006          25.3          -97.49%
BenchmarkValidateVersionTilde-8            1023          27.6          -97.30%
BenchmarkValidateVersionUnary-8            1034          29.0          -97.20%
BenchmarkCheckVersionWildcard-8            1054          42.0          -96.02%
BenchmarkValidateVersionWildcard-8         1086          50.5          -95.35%
BenchmarkValidateVersionRangeFail-8        3278          1238          -62.23%
BenchmarkValidateVersionUnion-8            3340          1336          -60.00%
BenchmarkValidateVersionTildeFail-8        2294          1257          -45.20%
BenchmarkValidateVersionCaretFail-8        2288          1272          -44.41%
BenchmarkValidateVersionUnaryFail-8        2266          1274          -43.78%
BenchmarkValidateVersionUnionFail-8        4627          2610          -43.59%
BenchmarkValidateVersionWildcardFail-8     2325          1314          -43.48%

And the memory allocation:

BenchmarkCheckVersionUnary-8               10             0              -100.00%
BenchmarkCheckVersionTilde-8               10             0              -100.00%
BenchmarkCheckVersionCaret-8               10             0              -100.00%
BenchmarkCheckVersionWildcard-8            10             0              -100.00%
BenchmarkCheckVersionRange-8               20             0              -100.00%
BenchmarkCheckVersionUnion-8               20             0              -100.00%
BenchmarkValidateVersionUnary-8            10             0              -100.00%
BenchmarkValidateVersionUnaryFail-8        20             10             -50.00%
BenchmarkValidateVersionTilde-8            10             0              -100.00%
BenchmarkValidateVersionTildeFail-8        20             10             -50.00%
BenchmarkValidateVersionCaret-8            10             0              -100.00%
BenchmarkValidateVersionCaretFail-8        20             10             -50.00%
BenchmarkValidateVersionWildcard-8         10             0              -100.00%
BenchmarkValidateVersionWildcardFail-8     20             10             -50.00%
BenchmarkValidateVersionRange-8            20             0              -100.00%
BenchmarkValidateVersionRangeFail-8        30             10             -66.67%
BenchmarkValidateVersionUnion-8            30             10             -66.67%
BenchmarkValidateVersionUnionFail-8        40             20             -50.00%

So yeah, drop to zero allocs, plus a two order of magnitude speedup. I'll worry about optimizing the errors to not actually evaluate strings over in #7, but avoiding string evaluation is the single most important performance change we can make.

@mattfarina mattfarina merged commit 1bdc6b3 into Masterminds:master Apr 5, 2016
@mattfarina
Copy link
Member

@sdboyer great catch. Benchmarks for the win!

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.

None yet

2 participants