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

go api: reduce allocations in deck decompression #19

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

zdylag
Copy link
Contributor

@zdylag zdylag commented Oct 10, 2024

Previously we would allocate quite often in order to decompress and
parse the deck strings that were given to us. This could be alleviated by
precomputing some of the regular expressions, preallocating, and using
manual conversion logic instead of relying on json.

This could obviously be taken much further, but this seemed like a good
compromise on benefit while still retaining readability.

Benchmark comparison:

goos: linux
goarch: amd64
pkg: github.com/MaT1g3R/slaytherelics/api
cpu: AMD Ryzen 9 5900X 12-Core Processor
                              │  before.txt  │              after.txt              │
                              │    sec/op    │   sec/op     vs base                │
DecompressDeck/Simple_deck-24    7.934µ ± 1%   1.318µ ± 0%  -83.39% (p=0.000 n=10)
DecompressDeck/Big_deck-24      227.58µ ± 1%   60.34µ ± 1%  -73.48% (p=0.000 n=10)
geomean                          42.49µ        8.916µ       -79.02%

                              │  before.txt  │              after.txt               │
                              │     B/op     │     B/op      vs base                │
DecompressDeck/Simple_deck-24    5494.5 ± 0%     836.0 ± 0%  -84.78% (p=0.000 n=10)
DecompressDeck/Big_deck-24      330.8Ki ± 0%   219.2Ki ± 0%  -33.72% (p=0.000 n=10)
geomean                         42.13Ki        13.38Ki       -68.24%

                              │ before.txt  │             after.txt              │
                              │  allocs/op  │ allocs/op   vs base                │
DecompressDeck/Simple_deck-24    94.00 ± 0%   17.00 ± 0%  -81.91% (p=0.000 n=10)
DecompressDeck/Big_deck-24      1810.0 ± 0%   165.0 ± 0%  -90.88% (p=0.000 n=10)
geomean                          412.5        52.96       -87.16%

zdylag added 3 commits October 9, 2024 22:44
This utility is pretty handy for comparing benchmarks in a more human
friendly way.
This file previously did not have tests, which is probably fine, but since
I wanted to change it a bunch and have not worked with it adding tests
first seemed like a safer thing to do.

This also adds a benchmark which can show us how expensive this is
right now. The current output looks like:
```
goos: linux
goarch: amd64
pkg: github.com/MaT1g3R/slaytherelics/api
cpu: AMD Ryzen 9 5900X 12-Core Processor
                              │ before.txt  │
                              │   sec/op    │
DecompressDeck/Simple_deck-24   7.934µ ± 1%
DecompressDeck/Big_deck-24      227.6µ ± 1%
geomean                         42.49µ

                              │  before.txt  │
                              │     B/op     │
DecompressDeck/Simple_deck-24   5.366Ki ± 0%
DecompressDeck/Big_deck-24      330.8Ki ± 0%
geomean                         42.13Ki

                              │ before.txt  │
                              │  allocs/op  │
DecompressDeck/Simple_deck-24    94.00 ± 0%
DecompressDeck/Big_deck-24      1.810k ± 0%
geomean                          412.5
```
Previously we would allocate quite often in order to decompress and
parse the deck strings that were given to us. This could be alleviated by
precomputing some of the regular expressions, preallocating, and using
manual conversion logic instead of relying on json.

This could obviously be taken much further, but this seemed like a good
compromise on benefit while still retaining readability.

Benchmark comparison:
```
goos: linux
goarch: amd64
pkg: github.com/MaT1g3R/slaytherelics/api
cpu: AMD Ryzen 9 5900X 12-Core Processor
                              │  before.txt  │              after.txt              │
                              │    sec/op    │   sec/op     vs base                │
DecompressDeck/Simple_deck-24    7.934µ ± 1%   1.318µ ± 0%  -83.39% (p=0.000 n=10)
DecompressDeck/Big_deck-24      227.58µ ± 1%   60.34µ ± 1%  -73.48% (p=0.000 n=10)
geomean                          42.49µ        8.916µ       -79.02%

                              │  before.txt  │              after.txt               │
                              │     B/op     │     B/op      vs base                │
DecompressDeck/Simple_deck-24    5494.5 ± 0%     836.0 ± 0%  -84.78% (p=0.000 n=10)
DecompressDeck/Big_deck-24      330.8Ki ± 0%   219.2Ki ± 0%  -33.72% (p=0.000 n=10)
geomean                         42.13Ki        13.38Ki       -68.24%

                              │ before.txt  │             after.txt              │
                              │  allocs/op  │ allocs/op   vs base                │
DecompressDeck/Simple_deck-24    94.00 ± 0%   17.00 ± 0%  -81.91% (p=0.000 n=10)
DecompressDeck/Big_deck-24      1810.0 ± 0%   165.0 ± 0%  -90.88% (p=0.000 n=10)
geomean                          412.5        52.96       -87.16%
```
@MaT1g3R MaT1g3R merged commit caf7d31 into Spireblight:master Oct 10, 2024
1 check passed
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