DEFLATE analysis and the --analyze report - #12
Conversation
Every entry point required a `ReadAt` source, so gzip arriving on a pipe could not be decoded at all. Path 1 only ever moves forward through the compressed bytes, so extract the operations it uses into an internal `InputCursor` trait and implement it for both the positional `SourceCursor` and a new forward-only `StreamCursor`. Member framing, footer verification, trailing-garbage detection, and the output limit are then literally the same code for a stream as for a file, rather than a second implementation that could drift. Adds `Decoder::decode_stream` and `Decoder::stream_reader`, mirroring `Decoder::decode` and `Decoder::reader`. `Decoder::open` routes a path that cannot be read positionally to the streaming driver, which turns a call that failed before into one that succeeds. The CLI accepts `-` for standard input and follows the same routing. The streaming runtime is configured with one worker so the telemetry reports the concurrency actually in use. A streaming reader does not join its coordinator on drop, because that coordinator can be parked in a read against a producer that never writes again. No new dependency and no new unsafe code. Refs COMBINE-lab#6 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every case is fed through a source that implements only `Read`, so none of them can silently fall back to positional reads the way a `Cursor` would, and each is compared against the positional decode of the same bytes. Covers single-member, concatenated and empty members, BGZF with its EOF member, a fully stored stream, truncation inside DEFLATE and inside the footer, a corrupt CRC32 and a corrupt ISIZE naming the offending member, trailing garbage, the output limit, telemetry, `Box<dyn Read + Send>` through paraseq, dropping a reader against a stalled producer, and a slow producer writing in small chunks. On unix it also decodes a real `cat` pipe and checks that `Decoder::open` routes a FIFO to the streaming path while leaving a regular file on a parallel one. Refs COMBINE-lab#6 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records what streaming input supports, what it does not, and why: the same verification as a regular file because it is the same sequential code, but no parallelism because every parallel path needs positional reads. ARCHITECTURE.md explains that the length snapshot is absent for a stream rather than mutable, that end of input plus the existing trailing-garbage check is what makes it authoritative, and why a streaming reader does not join on drop. SAFETY.md is unchanged; this feature added no unsafe code. Refs COMBINE-lab#6 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading into an empty buffer tail returns `Ok(0)`, which the cursor would read as end of input and turn into silent truncation. Callers only refill a drained window so this is unreachable today, but the failure mode is bad enough to be worth closing. Refs COMBINE-lab#6 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
First of four sub-projects reimplementing PR COMBINE-lab#5 in reviewable pieces. Covers index construction as a by-product of decoding, the native, GZIDX, .gzi, and gztool on-disk formats, and a separate IndexedReader for random access. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Thirteen tasks from index types through the four on-disk formats, the IndexedReader, decode-path wiring, interop tests, and documentation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The wrapper keeps the crate's inflate-side unsafe in one place and now also accepts a predecessor window as a plain byte slice, which the indexed reader needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Index building is opt-in through DecoderBuilder::build_index and arrives in DecodeReport::index. Member starts are offered by the sequential and streaming paths, which every source reaches. DecodeReport is no longer Copy because it can now carry an index. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Advanced SIMD arm always returns, which made the scalar fallback unreachable on AArch64, and the mask load had no safety comment. Both are invisible on x86_64, where CI runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The estimated-grid path offers each chunk start with its resolved predecessor window, which is where interior, non-byte-aligned resume points come from. BGZF blocks are independent members, so every non-empty block start is offered with no window, using each block's ISIZE footer for its decompressed offset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six ignored tests exercise both directions for bgzip, indexed_gzip, and gztool, with a CI job that installs all three. The first run found a real bug: a checkpoint without a predecessor window is not necessarily a member start, since indexed_gzip records its first point at the DEFLATE start, so the reader now detects a gzip header instead of assuming one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
deflateBound takes a c_ulong, which is 32 bits on Windows, and the pinned gztool tag does not exist; the project's tags reach v1.8.2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A single-stream loop over InputCursor covers both containers for positional and non-seekable sources alike. Format selection is explicit through DecoderBuilder::format, with auto-detection between gzip and zlib; raw DEFLATE has no header and must be requested. Raw streams accept an optional expected decompressed size, the only end-to-end check that format allows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both containers are a single DEFLATE stream, so the estimated-grid path handles them by starting at the format's DEFLATE offset and verifying the format's trailer where a gzip footer would go. Accounting keeps an Adler-32 for zlib instead of a CRC32. Index checkpoints record the DEFLATE start, as indexed_gzip does, so IndexedReader resumes there without change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sequential gzip loop, the single-stream zlib and raw DEFLATE loop, and the BGZF block decoder now inflate through InflateBackend, so an alternative implementation plugs in without touching a call site. The marker/window path and IndexedReader keep the concrete zlib-rs inflater, because they resume at arbitrary bit offsets and need zlib's Z_BLOCK contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The member loop still drove zlib-rs directly, so it was not actually pluggable. It now goes through the trait like the other whole-stream paths. Making that work exposed a hole in the abstraction: `RawInflater` kept inherent `new`, `reset`, and `message` alongside its trait methods, so every call site resolved to the inherent version and the trait was dead code. Those three now live only in the trait impl. What stays inherent is what the trait deliberately omits, `prime` and the two dictionary calls, which belong to the paths that resume mid-stream. Drop `set_dictionary` from the trait for the same reason: no whole-stream path installs a window, since each stream it inflates starts at its own beginning. Backends report DEFLATE errors without a stream position, having seen only the buffer they were handed. `at_bit_offset` lets the call site, which knows the position, fill it in.
`isal_backend.rs` implements the backend trait over ISA-L, so the whole-stream paths decode through it when the feature is on. The default build is untouched: `isal-sys` is an optional dependency and nothing else changed. ISA-L is linked, not vendored. `use-system-isal` and `shared` keep the build from compiling the library from source, which would need autotools and an assembler. Two details the C interface forces: `inflate_state` embeds a 64 KiB scratch buffer, so it is boxed rather than held inline. It is zeroed before `isal_inflate_init`, which sets every scalar but leaves the scratch buffers alone. ISA-L reads ahead into a bit buffer, so at the end of a stream it has taken input the stream does not own. Whole bytes still in that buffer are given back, otherwise the gzip footer and the next member start at the wrong offset. The concatenated-member and BGZF tests are what catch this. The full suite passes with `--features isal`, including the indexed-seek tests, which is the check that the bit-accurate paths still run on zlib-rs.
A job installing libisal-dev, then running clippy and the suite with the feature on. The default jobs cannot reach this code: the feature is off and the library is not vendored.
`benches/inflate_backend.rs` decodes a 16 MiB corpus single-threaded through the two pluggable paths reachable from a byte slice. The backend is a compile-time choice, so comparing means two runs against a criterion baseline; the benchmark identifiers stay backend-agnostic so those runs line up. The corpus carries pseudo-random fields. Verbatim-repeating text compresses into a few very long matches that both backends copy at memory speed, which hid the Huffman decoding entirely: an earlier draft measured 3.9 GiB/s and would have compared nothing. On Apple M-series against isa-l 2.32.1, ISA-L is 20% slower on both paths, well outside the noise. That is the finding, and it is recorded rather than buried. The CI job now also runs both benchmarks on x86-64, where ISA-L's assembly decoder is strongest and where the project has no local hardware to measure. The feature stays off by default either way. Documentation covers the split in ARCHITECTURE.md, the ISA-L unsafe argument in SAFETY.md, the measurement in PERFORMANCE_AUDIT.md, and how to enable and reproduce in README.md and the crate docs. SAFETY.md also gets a stale path corrected: the zlib wrapper moved to inflate.rs.
Sub-project 4 splits in two. Reproducing rapidgzip's --analyze needs a block walker in core plus a byte-exact reimplementation of its report; the option surface needs none of that. Together they would be a pull request too large to review, which is the failure this whole exercise exists to avoid. Designing the CLI surfaced a real bug: Checkpoint::line_offset and GzipIndex::total_line_count are read by the gztool importer and written as zero by every decode path, so a gztool-with-lines index exported from here would be silently wrong. Line counting lands in 4a, where -l and line-addressed ranges need it anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`DecoderBuilder::count_lines` fills `DecodeReport::line_count` and, when an index is also collected, every checkpoint's line offset plus the index's total. This closes a latent bug: `Checkpoint::line_offset` and `GzipIndex::total_line_count` were read by the gztool importer and written as zero by every decode path, so a gztool-with-lines index exported from here would have been silently wrong. Counting happens in the `Output` implementations, on the thread that emits. That is the only place the bytes are final: the marker path's chunks hold 16-bit symbols until the coordinator resolves them, and a marker can resolve to a newline. Checkpoint line offsets come from merging two ordered streams. The builder keeps the offsets it has been offered but not yet passed, and each run of output resolves the ones it covers in a single scan. Offers always precede the emit of the bytes at their offset, so nothing is passed before it is known. Rather than trust that, the builder tracks whether every checkpoint was resolved and claims a total line count only when they all were, so a future path that offers late degrades to an index without line counters instead of one full of zeros. The tests check the claim the format actually makes: each checkpoint's line offset equals the newlines preceding its decompressed offset, on both the marker grid and BGZF. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`IndexedReader::seek_to_line` resumes at the checkpoint at or before a zero-based line and scans forward, so at most one checkpoint spacing of output is read regardless of file size. It refuses an index without line counters rather than scanning from the start, which would defeat the point of having an index. Writing gztool's line-aware format now refuses an index that never counted lines. Writing zeros produced a file gztool accepts and then trusts, which is worse than refusing. Adding an interop test where real gztool extracts by line found two bugs in the export that the existing byte-offset test could not see. gztool numbers lines from one: its first point reads L1 with no preceding newline. Ours count newlines before the point. Export and import now convert. More seriously, a member checkpoint recorded the member start, so gztool resumed by inflating raw DEFLATE at the gzip header and reported "Compressed data error". indexed_gzip and gztool both record where DEFLATE begins, and our own reader already tolerated either, having been taught to skip a header it finds at a checkpoint. Member checkpoints now record the DEFLATE start too. BGZF block checkpoints keep recording the block start, which is what the .gzi format means by an offset. The existing byte-offset test missed this because it only ever seeks to an interior checkpoint, which carries a window and a bit offset and was always correct. Only the empty-window member points were wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every option rapidgzip 0.16.0 accepts is now accepted here, with the same short and long names, so a command line written for that tool works. The work already in the library, indexes in five formats, seeking, zlib and raw DEFLATE, becomes reachable without writing Rust. main.rs kept only arguments and dispatch. Input classification and output destination went to source.rs, index formats to index.rs, range parsing and extraction to ranges.rs, and result printing to report.rs. The two pieces with real logic, range parsing and output path derivation, are pure functions with unit tests next to them, so the integration tests do not have to enumerate their edge cases. Three options are accepted no-ops, because they name behaviour this crate does not have: --io-read-method, which has one strategy here, and --sparse-windows with its negation, since index windows are always dense. A dense index is valid and interoperable, merely larger, so accepting the flag lies about nothing. --no-verify is refused instead. Verification is structural here: a member is accepted only after its CRC32 and size check out. Accepting the flag would promise a speedup that does not exist. Two more refusals replace silent surprises: --import-index without --ranges, where the index would go unused, and --index-format gztool-with-lines without --count-lines, where the format has nowhere to get its counters. Output goes where rapidgzip sends it, including the derived name with the compressed suffix stripped, and an existing file is kept unless --force says otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
README.md gains the option examples, the index-format list, and a section naming the three deliberate differences from rapidgzip: the accepted no-ops, the refusal of --no-verify, and that this tool always decodes rather than skipping when output goes to /dev/null. ARCHITECTURE.md explains where line counting happens and why it cannot happen in the workers, plus how checkpoint line offsets are resolved. The crate docs cover count_lines and seek_to_line, and record that a checkpoint names the DEFLATE start rather than a container header, with BGZF the exception the .gzi format requires. CHANGELOG.md gains a Fixed section for the two gztool bugs, which is the first entry that section has needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Decoder::analyze` returns an `Analysis`: per stream the container, its header fields, offsets and footer; per block the encoding, offsets to the bit, sizes, the three declared Huffman alphabets, how much output came from literals against copies, and how far its back-references reach into the preceding window. The walk reuses `parallel/deflate.rs`, which already parses dynamic headers and decodes symbols. It needed the code lengths that parsing discards, so `dynamic_trees_with_lengths` hands them back, and the building blocks became `pub(crate)`. The decode loop is the analyzer's own, since it counts symbol kinds and back-reference distances rather than producing markers. Header fields are parsed here rather than by `parse_member_header`, which deliberately keeps only the offsets the decoder needs. Checked against real rapidgzip 0.16.0 on the same file: every field agrees, including the ones a subtly wrong walk would get close but not exact, the farthest back-reference, the merged reference count, and the used window symbol count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The report is meant to be byte-identical to rapidgzip 0.16.0, which means
printing numbers the way C++ streams do rather than the way Rust does:
`%.6g` by default, `%.6e` under std::scientific, bit counts as
"{bytes} B {bits} b", and byte counts as a sum of binary units so 31908
prints as "31 KiB 180 B" instead of being rounded into one.
The histogram reproduces rapidgzip's bucketing exactly, including the two
rules that are easy to miss: an integral range narrower than the bin
count shrinks the bin count, and the maximum value lands in the last bin
rather than one past the end.
These are pure functions with their own tests. A rounding rule that is
wrong only at an edge case would otherwise surface as an opaque diff in
the differential test, which is a much worse place to debug it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--analyze` reproduces rapidgzip 0.16.0's report: stream headers, one section per DEFLATE block, footers, alphabet statistics, the code-length and block-size distributions, the back-reference length table, and the block type counts. Diffed against the real tool on a 754 KiB gzip file, 974 lines of report, 28 lines differ and they fall into exactly two groups. Twenty-one are the benchmark profile, which prints wall-clock durations. Ours carries its own measurements, and the differential test masks them. The walk does not separate header parsing from symbol decoding, so the five-way split reports zero rather than a fabricated division. Seven are "Number of merged back-references", one lower than the reference in seven of thirty-one blocks. The back-reference count itself matches everywhere, so the sets agree and only the merge disagrees. The reference sorts by distance with std::sort, which is not stable, then merges pairwise; sorting by distance and then length does not reproduce its result. This is recorded rather than papered over, and is the one field of the report that is not yet byte-identical. Two sections rapidgzip contains are deliberately not printed. Its back-reference-length and window-symbol histograms are guarded on a counter its histogram type never updates, so neither can ever print. Emitting them would be a difference, not an improvement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`tests/analyze_interop.rs` runs rapidgzip 0.16.0 and ours over the same file and diffs, on a single member, a stored member, concatenated members, and a six-byte file. The interop CI job installs the reference from PyPI. Every line must match except two, both stated in README.md. The benchmark profile prints wall-clock durations, so ours carries its own measurements. `Number of merged back-references` is now a plain interval union. The reference merges pairwise after an unstable sort, assigning the current run the following reference's end even when that reference is contained, which shortens the run. Its result therefore depends on how equal distances happened to be ordered, so it is not a function of the file and cannot be reproduced by construction. Sorting by distance then length ascending and descending both fail to match it, in opposite directions, which is what confirmed the cause. Ours is deterministic and, where they disagree, correct. Two sections the reference contains stay unprinted: its back-reference-length and window-symbol histograms are guarded on a counter its histogram type never updates, so neither can ever appear. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Status update after the current-main review and #18 The analyzer is a worthwhile feature, especially as a diagnostic for unusual DEFLATE structure, but this stacked PR should not be merged into current main. Its core feature can be implemented independently of the CLI-parity work in #11 and independently of #18; the final The clean design should retain the strong parts of this proposal—one authoritative block walk, structured library output, multi-format/multi-member coverage, and differential checks against C++ rapidgzip—while changing the following points. Recommended API and resource model
Reference-output compatibilityThe CLI should target rapidgzip 0.16.0's report where that report is a deterministic function of the input, with fixtures for exact bytes. Known reference defects should be normalized and documented rather than copied into the public data model. In particular, deterministic interval union is preferable to reproducing unstable equal-distance sort behavior for merged back-references. The compatibility test should mask or compare separately:
That makes the claim precise: deterministic sections match, measured sections have matching shape, and deliberate corrections are named. Clean implementation sequence
#18 does not need analyzer code. In the future, the same back-reference summaries may inform a real sparse-window index transformation, but that should be a separate measured feature rather than coupling analysis to indexed full-stream decoding now. I suggest keeping this PR as the reference-output research artifact until a clean core-analysis PR is opened, then closing the stack as superseded. |
|
Superseded by #20, which reimplements the structural-analysis feature cleanly from the current The replacement keeps the useful analysis and compatible Closing this stacked version so review and CI can focus on the current implementation. Thank you for the original design and compatibility work; it directly informed the replacement. |
Sub-project 4b of 4, the last one, replacing #5. Stacked on #11, which stacks on #10, #9, #8. GitHub cannot base a cross-fork pull request on a fork branch, so this targets
mainand its diff contains all four parents; review the top four commits only. Merge #8, #9, #10, and #11 first.What this adds
Decoder::analyzewalks every DEFLATE block and returns anAnalysis: per stream the container, its header fields, offsets and footer; per block the encoding, offsets to the bit, sizes, the three declared Huffman alphabets, how much output came from literals against copies, and how far its back-references reach into the preceding window.--analyzeprints it in rapidgzip 0.16.0's exact layout.The walk reuses
parallel/deflate.rs, which already parses dynamic headers and decodes symbols. It needed the code lengths that parsing discards, sodynamic_trees_with_lengthshands them back. The decode loop is the analyzer's own, since it counts symbol kinds and back-reference distances rather than producing markers.How byte-exact it actually is
Diffed against the real tool on a 754 KiB gzip file: 974 lines of report, and only two things differ.
The benchmark profile prints wall-clock durations. Ours carries its own measurements, so the section is there with the same labels and shape but different numbers.
Number of merged back-referencesdiffers in some blocks, and this one is worth explaining because it is not a bug on our side. The reference sorts references by distance withstd::sort, then merges pairwise, assigning the current run the following reference's end even when that reference is contained, which shortens the run. Its result therefore depends on how its unstable sort ordered equal distances, so it is not a function of the file. Sorting by distance then length ascending and descending both fail to match it, in opposite directions, which is what confirmed the cause. Ours is a plain interval union: deterministic, and where they disagree, correct.Everything else matches byte for byte, including the parts most likely to be subtly wrong:
31 KiB 180 Bstyle byte formatting,%.6gand%.6enumber formatting, and the histogram bucketing with its two easy-to-miss rules (an integral range narrower than the bin count shrinks the bin count, and the maximum lands in the last bin).Two sections the reference contains are deliberately not printed. Its back-reference-length and window-symbol histograms are guarded on a counter its histogram type never updates, so neither can ever appear in real output. Emitting them would be a difference, not an improvement.
Verification
tests/analyze_interop.rs, ignored by default, diffs against real rapidgzip 0.16.0 on a single member, a stored member, concatenated members, and a six-byte file. The interop CI job installs it from PyPI.This completes the replacement of #5
Four sub-projects, five pull requests: #8 index and seeking, #9 zlib and raw DEFLATE, #10 pluggable inflate backends, #11 command-line parity, and this one.