Repository navigation
fix(encoding): single-segment frames whenever the window covers the content - #591
Conversation
…ontent The streaming encoder wrote a single-segment frame only for pledged sizes of 512 bytes to 16 KiB and never with a dictionary, and the frame compressor only when a size was hinted before the frame. Upstream's rule (`ZSTD_writeFrameHeader`) is one condition: the content size is written and the window covers it. Elsewhere our frames declared the power of two above the content as their window, so a decoder reserved more memory than the data needs: a 140 MiB file at `--long=28` asked for 256 MiB where upstream's frame asks for 140 MiB. - One rule in `frame_header::single_segment`, read by both encoders, dictionary frames included (RFC 8878 3.1.1.1.2: the window is the content size, and the dictionary stays referenceable before it). - The CLI's frames change in the header alone: everything after it is byte-identical at levels 1-19 on four fixture sizes, with and without a dictionary, and the header is up to four bytes shorter. Tests check both encoders against libzstd's header layout from 100 bytes to 1 MiB with and without a dictionary, with a C round-trip; two tests that read the window from the header now use content past it. Closes #570
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
ZSTD_writeFrameHeader).--long=28declares 140 MiB instead of 256 MiB.Changes
frame_header::single_segmentholds the rule; the streaming encoder and the frame compressor both call it.Testing
Unit, CLI, ffi (header layout against libzstd from 100 bytes to 1 MiB with and without a dictionary, C round-trip) and C ABI tests, clippy over the CI feature sets, formatting, rustdoc and doc tests pass on macOS.
Closes #570
Summary by CodeRabbit