viirya opened a new pull request, #2558:
URL: https://github.com/apache/iceberg-rust/pull/2558

   ## Which issue does this PR close?
   
   - Closes #2557.
   
   ## What changes are included in this PR?
   
   Adds a criterion-based benchmark harness for the `ArrowReader` at 
`crates/iceberg/benches/arrow_reader.rs`. Until now there were no in-repo 
reader benchmarks, so every performance claim in the perf epic (#2172) had to 
be validated against external workloads. This harness writes Parquet files to a 
local temp dir and reads them back through the normal `FileIO` path, measuring 
the per-`FileScanTask` overhead that dominates scans of tables with many small 
files. Because it runs on the local FS, it isolates CPU / per-task work rather 
than network latency.
   
   Benchmark groups, chosen to map onto the epic's code paths:
   
   - **many_small_files** — scans 16/64/256 small files, reporting files/sec so 
per-file overhead is directly visible.
   - **concurrency** — a fixed corpus read at concurrency 1/4/16, exercising 
both the single-concurrency fast path and the buffered/flattened multi-task 
path.
   - **migrated_table** — files without embedded field IDs read via name 
mapping, isolating the migrated-table schema-resolution cost (#2176 path).
   - **same_file_splits** — one multi-row-group file read as 1/8/32 byte-range 
tasks, surfacing the redundant per-split metadata fetch that the same-file 
metadata caching proposed in #2172 (attempted in #2100) targets.
   - **with_predicate** — scans carrying a bound predicate with row-group 
filtering and row selection enabled, exercising the per-task row-filter setup.
   
   This adds `criterion` (with the `async_tokio` feature) as a workspace 
dev-dependency and a `[[bench]]` entry to the `iceberg` crate.
   
   ### Measuring `bytes_read`, not just time
   
   Wall-clock time on the local FS cannot show the benefit of optimizations 
that save *round-trips* (e.g. the metadata size hint #2173 or range coalescing 
#2181) — those need a latency-bearing backend, which is left as follow-up. But 
the same-file-split problem is different: its cost is redundant *bytes 
fetched*, not latency, so it is measurable even locally.
   
   The `same_file_splits` group therefore reports `ScanMetrics::bytes_read` as 
its throughput basis (a deterministic count, not a sampled measurement) and 
prints the read amplification per split count. On a ~1.2 MiB file:
   
   ```
   same_file_splits/1:  bytes_read = 1,694,258  (1.43x file size)
   same_file_splits/8:  bytes_read = 5,529,051  (4.66x file size)
   same_file_splits/32: bytes_read = 18,674,303 (15.72x file size)
   ```
   
   Reading one file as 32 byte-range splits fetches ~15.7x the file size, 
because each split re-fetches the Parquet metadata independently. This is 
machine-independent evidence for the cost that same-file metadata caching 
targets — #2100 was closed partly because its author could not demonstrate a 
benefit on their workload (a ~1:1 task-to-file table, where same-file caching 
has nothing to hit); with this benchmark a caching implementation would show 
the amplification dropping back toward 1x.
   
   Run with:
   
   ```
   cargo bench -p iceberg --bench arrow_reader
   ```
   
   ## Are these changes tested?
   
   This is a benchmark, not a code change. The harness compiles and runs 
(`cargo bench -p iceberg --bench arrow_reader`), and reuses the same 
Parquet-writing and reader-driving patterns as the existing reader unit tests. 
`cargo fmt` and `cargo clippy -p iceberg --benches` are clean.
   


-- 
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]

Reply via email to