feat: add ZipFileBuilder for parallelizing compression - #911
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “prepare then append” workflow to support parallelizing per-file compression work outside of ZipWriter, while still producing correct ZIP metadata when appending the pre-compressed payloads.
Changes:
- Introduces
ZipFileBuilderto compress a single file entry into memory while tracking CRC-32 and sizes, producing aPreparedZipFile. - Adds
ZipWriter::add_prepared_fileto append aPreparedZipFileby copying its already-compressed bytes verbatim (including handling ZIP64 header patching and stream/data-descriptor mode). - Adds tests covering roundtrip reading, duplicate-name rejection (and writer recovery), stream mode behavior, and encryption rejection.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Its-Just-Nans
left a comment
There was a problem hiding this comment.
Hi,
Thanks for the MR, a little question in comment.
Also, what do you think about adding a test with a thread (like the documentation)?
| crc32: u32, | ||
| uncompressed_size: u64, | ||
| compressed_size: u64, | ||
| data: Vec<u8>, |
There was a problem hiding this comment.
Why not using a Box<[u8]> ?
and change line 2526
-let data = cursor.into_inner();
+let data = cursor.into_inner().into_boxed_slice();|
I originally had a test case with some threads (similar to the doc comment), but I removed it on the theory that the borrow checker meant it had to work :-) Happy to add it back. |
Compresses a file's contents independently of any ZipWriter, so that multiple files can be compressed on separate threads and then appended serially with ZipWriter::add_prepared_file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed, thanks! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/write.rs:1801
- add_prepared_file() writes the already-compressed payload via ZipWriter::write_all(), which unconditionally updates ZipWriterStats (CRC32 + byte count) on the compressed bytes (see ZipWriter::write at write.rs:761+). For prepared files the CRC/sizes are already known, so this does extra CPU work proportional to the compressed size and can partially defeat the purpose of off-thread compression. Writing directly to the inner writer avoids the redundant hashing and bookkeeping.
// start_entry leaves the inner writer as a bare Storer (a compression encoder is only
// installed by start_file*), so the already-compressed bytes pass through unchanged.
let result = self.write_all(&data);
self.ok_or_abort_file(result)?;
Skips ZipWriter::write's per-byte stats bookkeeping, which is redundant for pre-compressed data whose CRC and sizes are already known. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/write.rs:2332
- Doc comment reads as if encryption-free options would fail. It likely meant that creating a builder will fail when encryption is requested; adding a comma clarifies the intended meaning.
/// Encryption is not supported; pass options without encryption or creation will fail.
Compresses a file's contents independently of any ZipWriter, so that multiple files can be compressed on separate threads and then appended serially with ZipWriter::add_prepared_file.