Jefffrey commented on code in PR #10878:
URL: https://github.com/apache/arrow-rs/pull/10878#discussion_r3872683393


##########
parquet/benches/writer_overhead.rs:
##########
@@ -82,5 +86,49 @@ fn bench_writer_overhead(c: &mut Criterion) {
     }
 }
 
-criterion_group!(benches, bench_writer_overhead);
+/// Column counts for [`bench_writer_repeated_batches`]. Kept below the widest
+/// case in [`COLUMN_COUNTS`] so the benchmark stays a few hundred milliseconds
+/// per iteration.
+const REPEATED_COLUMN_COUNTS: &[usize] = &[1_000, 5_000];
+
+/// Number of batches written per file by [`bench_writer_repeated_batches`], 
and
+/// the number of rows in each of them.
+const REPEATED_BATCH_COUNT: usize = 32;
+const REPEATED_BATCH_ROWS: usize = 32;

Review Comment:
   might as well inline them (or at least bring them inside) if the comments 
state theyre meant to be used by `bench_writer_repeated_batches`



##########
parquet/benches/writer_overhead.rs:
##########
@@ -82,5 +86,49 @@ fn bench_writer_overhead(c: &mut Criterion) {
     }
 }
 
-criterion_group!(benches, bench_writer_overhead);
+/// Column counts for [`bench_writer_repeated_batches`]. Kept below the widest
+/// case in [`COLUMN_COUNTS`] so the benchmark stays a few hundred milliseconds
+/// per iteration.
+const REPEATED_COLUMN_COUNTS: &[usize] = &[1_000, 5_000];
+
+/// Number of batches written per file by [`bench_writer_repeated_batches`], 
and
+/// the number of rows in each of them.
+const REPEATED_BATCH_COUNT: usize = 32;
+const REPEATED_BATCH_ROWS: usize = 32;
+
+/// Measures the per-`write` (rather than per-column-writer) overhead of a wide
+/// schema, by writing many small batches into a single row group.
+///
+/// [`bench_writer_overhead`] writes one batch per file, so it is dominated by
+/// column writer construction and metadata assembly. This benchmark keeps the
+/// number of column writers fixed and increases the number of `write` calls,
+/// isolating the work that repeats on every batch.

Review Comment:
   this part about `bench_writer_overhead` should be moved to 
`bench_writer_overhead` instead of being here



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