Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

feat: Improve zstd encoder and decoder usage in the compressor codec #420

Merged
merged 1 commit into from
Jul 28, 2024
Merged

feat: Improve zstd encoder and decoder usage in the compressor codec #420

merged 1 commit into from
Jul 28, 2024

Conversation

bignacio
Copy link
Contributor

Addressing issue #419

benchmark results always creating a new zstd encoder/decoder

goos: linux
goarch: amd64
pkg: github.com/hamba/avro/v2/ocf
cpu: AMD Ryzen 5 3550H with Radeon Vega Mobile Gfx


BenchmarkZstdEncodeDecodeLowEntropyLong
BenchmarkZstdEncodeDecodeLowEntropyLong-8    	     289	   3523847 ns/op	10891887 B/op	      40 allocs/op
BenchmarkZstdEncodeDecodeHighEntropyLong
BenchmarkZstdEncodeDecodeHighEntropyLong-8   	     298	   3390952 ns/op	10894703 B/op	      40 allocs/op

benchmark results reusing an existing zstd encoder/decoder

BenchmarkZstdEncodeDecodeLowEntropyLong
BenchmarkZstdEncodeDecodeLowEntropyLong-8    	   55628	     22883 ns/op	   19220 B/op	       2 allocs/op
BenchmarkZstdEncodeDecodeHighEntropyLong
BenchmarkZstdEncodeDecodeHighEntropyLong-8   	   47652	     25064 ns/op	   31553 B/op	       3 allocs/op

Go CPU profiling always creating a new zstd encoder/decoder
Before
ocfpprof-before

After
ocfpprof-after

@bignacio
feat: working on improving zstd codec resource usage
d14c70e 
  - add tests and benchmark for zstd codec
@bignacio bignacio force-pushed the zstd_resource_usage branch from 2466fa8 to d14c70e Compare July 28, 2024 16:43
nrwiersma
nrwiersma approved these changes Jul 28, 2024
View reviewed changes
Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the contribution

@nrwiersma nrwiersma merged commit f8a0492 into hamba:main Jul 28, 2024
3 checks passed
@nrwiersma nrwiersma mentioned this pull request Jul 28, 2024
Closed
@bignacio bignacio deleted the zstd_resource_usage branch July 28, 2024 17:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@nrwiersma nrwiersma nrwiersma approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bignacio @nrwiersma