kevinjqliu opened a new pull request, #3468: URL: https://github.com/apache/iceberg-rust/pull/3468
## Which issue does this PR close? - Follow-up to #3354, whose review asked for a benchmark backing its parse speedup. ## What changes are included in this PR? Adds the repo's first benchmark, `crates/iceberg/benches/manifest_parse.rs` (criterion), for `Manifest::parse_avro`. It writes V2 data manifests with 1k, 10k, and 50k entries using `ManifestWriter`, so no fixtures or Python are needed. Each entry has stats for 12 columns and an identity partition on a string column, matching the `gen_manifests.py` script from #3354. There are two cases: - `same_writer_schema`: the manifest exactly as iceberg-rust writes it. - `fewer_writer_fields`: the same manifest rewritten without `first_row_id`, `content_offset`, and `content_size_in_bytes`, which PyIceberg doesn't write. The second case matters because apache-avro 0.21 (before #3354) only resolves schemas when the writer and reader schemas differ. That is the usual case for manifests written by PyIceberg or Java. The bench checks that both cases parse to the same `Manifest`. Run with `cargo bench -p iceberg --bench manifest_parse`. Results on an Apple M5 Max (criterion median, 10 samples). "Before #3354" is `ecba0b537` with this bench copied in; it needed a two-line local tweak for the 0.21 `Writer` API, which is not part of this PR. | case | entries | before #3354 | `main` | speedup | |---|--:|--:|--:|--:| | `fewer_writer_fields` | 1,000 | 26.5 ms | 4.17 ms | 6.4x | | `fewer_writer_fields` | 10,000 | 264.7 ms | 42.3 ms | 6.3x | | `fewer_writer_fields` | 50,000 | 1328 ms | 212.7 ms | 6.2x | | `same_writer_schema` | 1,000 | 10.8 ms | 4.32 ms | 2.5x | | `same_writer_schema` | 10,000 | 108.7 ms | 43.4 ms | 2.5x | | `same_writer_schema` | 50,000 | 550.0 ms | 215.7 ms | 2.5x | `ManifestWriter` writes uncompressed manifests, while the #3354 numbers used deflate, so absolute times differ from that PR. ## Are these changes tested? This PR only adds a benchmark. I ran: - `cargo bench -p iceberg --bench manifest_parse`: passed, numbers above. - `cargo clippy -p iceberg --all-targets --all-features -- -D warnings`: passed. - `cargo fmt --all -- --check`, `taplo fmt --check`, `cargo machete`, and `cargo deny check licenses bans`: all passed. ## AI Disclosure Written with GitHub Copilot (Claude Opus 5.5). Please check: - Using criterion as a workspace dev-dependency with `default-features = false` (no plotters or rayon). It adds 11 crates to `Cargo.lock` and moves `colored` and `winapi-util` to the newer `windows-sys` already in the lock. - Whether dropping those three optional fields is a fair stand-in for PyIceberg- or Java-written manifests. It is enough to make 0.21 resolve schemas, but other writers may differ in more ways. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
