You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Jefffrey (via GitHub)" <gi...@apache.org> on 2026/08/30 22:39:46 UTC
[PR] chore: cut down amount of zip benchmarks [arrow-rs]
Jefffrey opened a new pull request, #10919:
URL: https://github.com/apache/arrow-rs/pull/10919
current zip_kernels benchmark has 378 benches, which seems overkill
reducing down to 135 in this PR
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: cut down amount of zip benchmarks [arrow-rs]
Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on code in PR #10919:
URL: https://github.com/apache/arrow-rs/pull/10919#discussion_r3891662919
##########
arrow/benches/zip_kernels.rs:
##########
@@ -268,63 +262,23 @@ fn add_benchmark(c: &mut Criterion) {
},
);
- // Short strings
bench_zip_on_input_generator(
c,
&GenerateBytes::<GenericStringType<i32>> {
- description: "short strings (3..10)".to_string(),
+ description: "strings (3..20)".to_string(),
range_length: 3..10,
_marker: std::marker::PhantomData,
},
);
- // Long strings
- bench_zip_on_input_generator(
- c,
- &GenerateBytes::<GenericStringType<i32>> {
- description: "long strings (100..400)".to_string(),
- range_length: 100..400,
- _marker: std::marker::PhantomData,
- },
- );
-
- // Short Bytes
- bench_zip_on_input_generator(
- c,
- &GenerateBytes::<GenericBinaryType<i32>> {
- description: "short bytes (3..10)".to_string(),
- range_length: 3..10,
- _marker: std::marker::PhantomData,
- },
- );
-
- // Long Bytes
- bench_zip_on_input_generator(
- c,
- &GenerateBytes::<GenericBinaryType<i32>> {
- description: "long bytes (100..400)".to_string(),
- range_length: 100..400,
- _marker: std::marker::PhantomData,
- },
- );
-
bench_zip_on_input_generator(
c,
&GenerateStringView {
- description: "string_views size (3..10)".to_string(),
+ description: "string_views size (3..20)".to_string(),
range: 3..10,
Review Comment:
same here?
##########
arrow/benches/zip_kernels.rs:
##########
@@ -268,63 +262,23 @@ fn add_benchmark(c: &mut Criterion) {
},
);
- // Short strings
bench_zip_on_input_generator(
c,
&GenerateBytes::<GenericStringType<i32>> {
- description: "short strings (3..10)".to_string(),
+ description: "strings (3..20)".to_string(),
range_length: 3..10,
Review Comment:
should we increase the `range_length` to 20 as well?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: cut down amount of zip benchmarks [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10919:
URL: https://github.com/apache/arrow-rs/pull/10919#discussion_r3891984321
##########
arrow/benches/zip_kernels.rs:
##########
@@ -268,63 +262,23 @@ fn add_benchmark(c: &mut Criterion) {
},
);
- // Short strings
bench_zip_on_input_generator(
c,
&GenerateBytes::<GenericStringType<i32>> {
- description: "short strings (3..10)".to_string(),
+ description: "strings (3..20)".to_string(),
range_length: 3..10,
Review Comment:
whoops, thanks
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: cut down amount of zip benchmarks [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #10919:
URL: https://github.com/apache/arrow-rs/pull/10919#discussion_r3890742670
##########
arrow/benches/zip_kernels.rs:
##########
@@ -268,63 +262,23 @@ fn add_benchmark(c: &mut Criterion) {
},
);
- // Short strings
bench_zip_on_input_generator(
c,
&GenerateBytes::<GenericStringType<i32>> {
- description: "short strings (3..10)".to_string(),
+ description: "strings (3..20)".to_string(),
range_length: 3..10,
_marker: std::marker::PhantomData,
},
);
- // Long strings
- bench_zip_on_input_generator(
- c,
- &GenerateBytes::<GenericStringType<i32>> {
- description: "long strings (100..400)".to_string(),
- range_length: 100..400,
- _marker: std::marker::PhantomData,
- },
- );
-
- // Short Bytes
Review Comment:
removing bytes as strings covers same code paths (we dont do anything special for bytes)
##########
arrow/benches/zip_kernels.rs:
##########
@@ -200,11 +200,6 @@ fn bench_zip_on_input_generator(c: &mut Criterion, input_generator: &impl InputG
// Benchmarks for different scalar combinations
for (description, truthy, falsy) in &[
("null_vs_non_null_scalar", &null_scalar, &non_null_scalar_1),
- (
- "non_null_scalar_vs_null_scalar",
Review Comment:
this is essentially the same as the `null_vs_non_null_scalar` path, just without the predicate not operation:
https://github.com/apache/arrow-rs/blob/70fa5bcf21924c5a9ea34ee2df846ffae43f2bc6/arrow-select/src/zip.rs#L443-L457
##########
arrow/benches/zip_kernels.rs:
##########
@@ -268,63 +262,23 @@ fn add_benchmark(c: &mut Criterion) {
},
);
- // Short strings
bench_zip_on_input_generator(
c,
&GenerateBytes::<GenericStringType<i32>> {
- description: "short strings (3..10)".to_string(),
+ description: "strings (3..20)".to_string(),
range_length: 3..10,
_marker: std::marker::PhantomData,
},
);
- // Long strings
- bench_zip_on_input_generator(
- c,
- &GenerateBytes::<GenericStringType<i32>> {
- description: "long strings (100..400)".to_string(),
- range_length: 100..400,
- _marker: std::marker::PhantomData,
- },
- );
-
- // Short Bytes
- bench_zip_on_input_generator(
- c,
- &GenerateBytes::<GenericBinaryType<i32>> {
- description: "short bytes (3..10)".to_string(),
- range_length: 3..10,
- _marker: std::marker::PhantomData,
- },
- );
-
- // Long Bytes
- bench_zip_on_input_generator(
- c,
- &GenerateBytes::<GenericBinaryType<i32>> {
- description: "long bytes (100..400)".to_string(),
- range_length: 100..400,
- _marker: std::marker::PhantomData,
- },
- );
-
bench_zip_on_input_generator(
c,
&GenerateStringView {
- description: "string_views size (3..10)".to_string(),
+ description: "string_views size (3..20)".to_string(),
Review Comment:
removing short vs long benchmarks and just consolidating to a single one; maybe it can be handy to see effect of optimizations on short vs long strings specifically, but we dont have any codepath thats optimized for this anyway, as far as i can tell
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: cut down amount of zip benchmarks [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #10919:
URL: https://github.com/apache/arrow-rs/pull/10919
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org