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

groot/internal/rcompress: better buffer reuse #712

Open
sbinet opened this issue May 18, 2020 · 2 comments · May be fixed by #1023
Open

groot/internal/rcompress: better buffer reuse #712

sbinet opened this issue May 18, 2020 · 2 comments · May be fixed by #1023

Comments

@sbinet
Copy link
Member

sbinet commented May 18, 2020

we should consider introducing a better buffer+reader reuse strategy (e.g. for decompressor that support Reseting their readers)

this would reduce memory allocation and memory pressure.
this would then reduce the amount of work needed by the garbage collector.

issues:

  • as decompression/compression are tasks that greatly benefit from concurrency, we should devise a way not to limit it too drastically
  • races
  • for decompression, we need a mechanism to declare that a buffer is no longer in use (and a way to correctly do that)
  • consider using a type-parametrized sync.Map as proposed in proposal: sync, sync/atomic: add PoolOf, MapOf, ValueOf golang/go#47657
@bburghgr
Copy link

I stumbled upon this when playing around with some benchmarks that read a file using zlib compression, and a sync.Pool seems to work well enough. I think I can put something together for the various Resetters, if there's an interest.

In my toy benchmark, it reduces memory allocation (in my toy test) by about 60%, with a corresponding reduction in the number of GC cycles. Sadly, it doesn't seem to improve the event rate as much as I'd have hoped. On my machine, a file that needs ~30 seconds to process is sped up to ~28 seconds (wall time). I think I'm actually bottlenecked on something else, I'll open a separate issue describing what that may be.

@sbinet
Copy link
Member Author

sbinet commented Nov 15, 2024

yes, if you have a PR that exercizes this, it'd be great.
(bonus points if it comes w/ benchstat data from groot-bench)

from your comment, I suspect that's what I tried to achieve with sbinet/pzlib?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants