sdf-jkl opened a new pull request, #11298:
URL: https://github.com/apache/arrow-rs/pull/11298

   # Which issue does this PR close?
   
   N/A — small benchmark correction.
   
   # Rationale for this change
   
   `parquet_round_trip` clears its output buffer outside `b.iter`, so 
successive timed calls append complete files to the same Vec. Memory usage 
grows with Criterion's iteration count, and reallocations can affect the 
measurement. The subsequent read benchmark also inherits the accumulated bytes.
   
   # What changes are included in this PR?
   
   Move `buffer.clear()` inside the timed closure. Each iteration writes one 
file while retaining buffer capacity; the read benchmark receives the final 
single file. No library behavior changes.
   
   # Are these changes tested?
   
   - All 92 cases in the patched `parquet_round_trip` executable passed 
Criterion's `--test` smoke mode.
   - `rustfmt --check` and `git diff --check` passed.
   - Compared the actual benchmark executable at base 
`8208506f8f9ec193c08023ac2477d211d1d86d20` and this patch, using the same 
lockfile/dependencies. Linux x86-64, Ryzen AI 9 HX PRO 470, Rust 1.91.1, 
Criterion 0.8.2, optimized bench profile, pinned to CPU 2. Ran before / after / 
after / before, with 0.5 s warmup, 2 s measurement, 20 samples and 10,000 
bootstrap resamples per case.
   
   | Benchmark | Before (ms, runs 1 / 2) | After (ms, runs 1 / 2) | Change in 
mean |
   | --- | ---: | ---: | ---: |
   | `write String(20) plain` | 4.548 / 4.496 | 4.382 / 4.340 | -3.6% |
   | `write int32 dict` | 6.902 / 7.092 | 6.859 / 7.009 | -0.9% |
   | `write int32 plain` | 2.991 / 3.033 | 2.773 / 2.761 | -8.1% |
   | `read String(20) plain` | 2.276 / 2.267 | 2.308 / 2.333 | +2.2% |
   | `read int32 dict` | 1.011 / 1.068 | 1.005 / 0.991 | -4.0% |
   | `read int32 plain` | 0.529 / 0.527 | 0.516 / 0.506 | -3.1% |
   
   Peak RSS for the whole filtered benchmark process was **602.3 / 601.7 MiB 
before**, versus **84.8 / 84.6 MiB after** (`/usr/bin/time -v`). This includes 
fixture generation and the harness, not just the timed operation.
   
   These short local runs quantify the changed benchmark behavior, not a 
Parquet implementation speedup. Plain integer writes improved by about 8%; 
dictionary-write timings were close. Read results varied in both directions, 
including a 2.2% increase for the string case.
   
   Measurement arguments (same on both revisions):
   
   ```sh
   cargo bench -p parquet --bench parquet_round_trip -- \
     --warm-up-time 0.5 --measurement-time 2 --sample-size 20 \
     --nresamples 10000 --noplot \
     '^(write|read) (int32 (dict|plain)|String\(20\) plain)$'
   ```
   
   # Are there any user-facing changes?
   
   None; benchmark harness only.
   


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

Reply via email to