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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
