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

Some optimizations #58

Merged
merged 5 commits into from
Nov 29, 2024
Merged

Conversation

anatoly-kussul
Copy link
Contributor

@anatoly-kussul anatoly-kussul commented Nov 28, 2024

v4.0.0:

cpu: 12th Gen Intel(R) Core(TM) i5-12400F
BenchmarkUUID-12                  355501              3152 ns/op            2650 B/op         98 allocs/op
BenchmarkEncoding-12              414798              2828 ns/op            2640 B/op         98 allocs/op
BenchmarkDecoding-12             2422132               470.2 ns/op           152 B/op          5 allocs/op

with these optimizations:

BenchmarkUUID-12                 3777259               314.0 ns/op            48 B/op          2 allocs/op
BenchmarkEncoding-12             6792055               184.5 ns/op            32 B/op          1 allocs/op
BenchmarkDecoding-12             5202288               230.9 ns/op            64 B/op          2 allocs/op

@anatoly-kussul
Copy link
Contributor Author

fixed decoding benchmark, as it was returning error, and not fully decoding.

last commit benchmarks:

cpu: 12th Gen Intel(R) Core(TM) i5-12400F
BenchmarkUUID-12                 4058689               283.6 ns/op            40 B/op          2 allocs/op
BenchmarkEncoding-12             8116075               149.6 ns/op            24 B/op          1 allocs/op
BenchmarkDecoding-12             4644693               258.5 ns/op             0 B/op          0 allocs/op

v4.0.0 benchmarks with fixed decoding test:

cpu: 12th Gen Intel(R) Core(TM) i5-12400F
BenchmarkUUID-12                  363300              3179 ns/op            2650 B/op         98 allocs/op
BenchmarkEncoding-12              431427              2851 ns/op            2640 B/op         98 allocs/op
BenchmarkDecoding-12             1000000              1005 ns/op             192 B/op          8 allocs/op

@lithammer
Copy link
Owner

Very impressing work! 👏 You've managed to drastically reduce the number of allocations. Here's the output benchstat which shows the difference in percentage.

Nice find out error in the benchmark test case as well.

goos: darwin
goarch: arm64
pkg: github.com/lithammer/shortuuid/v4
cpu: Apple M2
           │   old.txt    │               new.txt               │
           │    sec/op    │   sec/op     vs base                │
UUID-8       2782.5n ± 1%   479.6n ± 0%  -82.77% (p=0.000 n=10)
Encoding-8   2438.5n ± 2%   216.5n ± 2%  -91.12% (p=0.000 n=10)
Decoding-8    368.6n ± 0%   266.9n ± 0%  -27.58% (p=0.000 n=10)
geomean       1.357µ        302.6n       -77.71%

           │   old.txt    │                 new.txt                 │
           │     B/op     │    B/op     vs base                     │
UUID-8       2650.00 ± 0%   40.00 ± 0%   -98.49% (p=0.000 n=10)
Encoding-8   2640.00 ± 5%   24.00 ± 0%   -99.09% (p=0.000 n=10)
Decoding-8     152.0 ± 0%     0.0 ± 0%  -100.00% (p=0.000 n=10)
geomean       1020.7                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

           │   old.txt   │                 new.txt                 │
           │  allocs/op  │ allocs/op   vs base                     │
UUID-8       98.000 ± 0%   2.000 ± 0%   -97.96% (p=0.000 n=10)
Encoding-8   98.000 ± 4%   1.000 ± 0%   -98.98% (p=0.000 n=10)
Decoding-8    5.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
geomean       36.35                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

@lithammer lithammer merged commit d3d4af7 into lithammer:master Nov 29, 2024
6 checks passed
@lithammer
Copy link
Owner

Included in v4.1.0. Thanks for your contribution!

@anatoly-kussul
Copy link
Contributor Author

Thanks for such quick approval and release!

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.

2 participants