bench: add deflate_many_small_files bench for #633 - #964
Conversation
Reproduces allocation blowup from zip-rs#633: 500x1KB DEFLATE 207MB vs TAR 326KB. Bench compares deflate vs stored for 10/100/500 files to track fix for reusing Compress::reset instead of re-allocating 380KB per entry. Run: cargo bench --bench deflate_alloc Related: zip-rs#633, zip-rs#723
There was a problem hiding this comment.
Pull request overview
Adds a new micro-benchmark intended to quantify and track heap allocation overhead when writing ZIP archives containing many small files, specifically comparing DEFLATE vs STORED to establish a baseline for addressing #633 and enabling regression tracking for #723.
Changes:
- Register a new
deflate_allocbenchmark target inCargo.toml. - Add a new benchmark (
benches/deflate_alloc.rs) that writes 10/100/500 1KB files usingZipWriterwith DEFLATE and STORED compression.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Cargo.toml | Registers the new deflate_alloc benchmark target. |
| benches/deflate_alloc.rs | Implements the benchmark comparing DEFLATE vs STORED for many small files. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let mut zip = ZipWriter::new(buf); | ||
| let opts = SimpleFileOptions::default().compression_method(CompressionMethod::Deflated); | ||
| for i in 0..n { |
| @@ -0,0 +1,41 @@ | |||
| use criterion::{criterion_group, criterion_main, Criterion, BenchmarkId, Throughput}; | |||
…terion - Use CompressionMethod::DEFLATE constant (always available) instead of CompressionMethod::Deflated (cfg _deflate-any) to compile with --no-default-features - Add criterion 0.5 to dev-dependencies (bench uses criterion, not bencher) per review: cargo bench --bench deflate_alloc failed without criterion
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
benches/deflate_alloc.rs:22
format!(...)creates aString, andZipWriter::start_fileimmediately callsto_string()internally, which clones/allocates again. That introduces avoidable per-entry allocations in the benchmark and can mask the effect you’re trying to measure. Also, without ablack_boxon the result, the compiler may be able to optimize away parts of the work.
This issue also appears on line 29 of the same file.
for i in 0..n {
zip.start_file(format!("file_{:04}.txt", i), opts).unwrap();
zip.write_all(&data).unwrap();
}
zip.finish().unwrap()
})
benches/deflate_alloc.rs:34
- Same benchmarking concern as the
deflatecase:format!(...)+start_file’s internalto_string()causes an extra allocation per entry, and returning the finished buffer withoutblack_boxrisks dead-code elimination affecting timings.
for i in 0..n {
zip.start_file(format!("file_{:04}.txt", i), opts).unwrap();
zip.write_all(&data).unwrap();
}
zip.finish().unwrap()
})
Related to #633, #723
Reproduces allocation blowup reported in #633:
91.7% from
zlib_rs::deflate::initviaCompress::new(~380KB per entry) + 7.9%flate2::zio::Writer.This bench compares
deflatevsstoredfor 10/100/500x1KB files to provide baseline for fixing #633 by reusingCompress::resetinstead of re-allocating perZipWriter::start_file.Run:
cargo bench --bench deflate_allocFollow-up fix will reuse compressor (see #633 suggested
Compress::reset). This bench gives tracking required by #723.