Repository navigation
zstd: avoid unused BuildDict encoder allocation - #1147
Conversation
BuildDict initialized a bestFastEncoder before checking whether BuildDictOptions.Level was set. When callers requested an explicit level, that encoder was immediately replaced by eOpts.encoder(), so the large best-compression tables were allocated and discarded without being used. Delay bestFastEncoder construction until the default-level path that actually uses it. This keeps the Level == 0 behavior unchanged while avoiding the unused encoder allocation for explicit levels. Add round-trip coverage for both BuildDict level paths and a benchmark that reports the allocation difference between default and explicit-level dictionary builds.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesBuildDict Encoder Initialization Refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Avoid constructing a
bestFastEncoderinBuildDictwhen callers provide an explicitBuildDictOptions.Level.The previous code allocated the default best-compression encoder before checking the requested level. For explicit levels, that encoder was immediately overwritten by
eOpts.encoder(), so its large tables were allocated and discarded without being used.This delays the best encoder construction until the default
Level == 0path, where it is still used. The PR also adds:TestBuildDictLevelPathsRoundTripfor default and explicit-level dictionary builds.BenchmarkBuildDictLevelPathsto report the allocation difference between those paths.Evidence
Correctness validation:
Allocation benchmark included in this PR:
Environment: Go 1.25.5, darwin/arm64, Apple M3.
For the baseline, the benchmark/test additions were run on
upstream/masterwithout thezstd/dict.goimplementation change.Explicit
SpeedFastestdictionary build:That is about 35.66 MB/op less allocation and one fewer allocation for explicit-level dictionary builds on this fixture. The default
Level == 0path remains around 72.26 MB/op, as expected, since it still constructs the best-compression encoder.Summary by CodeRabbit