Rich-T-kid commented on code in PR #27008:
URL: https://github.com/apache/datafusion/pull/27008#discussion_r4455862400
##########
datafusion/common/benches/with_hashes.rs:
##########
@@ -160,6 +160,33 @@ fn criterion_benchmark(c: &mut Criterion) {
do_hash_test(b, &arrays);
});
}
+
+ if name == "dictionary_utf8_int32" {
+ let dict = array
+ .as_any()
+ .downcast_ref::<DictionaryArray<Int32Type>>()
+ .expect("Expected Int32 dictionary");
+
+ for null_percent in [50, 97] {
+ let nullable_array = with_null_keys(dict, null_percent as f32
/ 100.0);
+
+ c.bench_function(
+ &format!("{name}: single, {null_percent}% null keys"),
+ |b| do_hash_test(b, std::slice::from_ref(&nullable_array)),
+ );
+ c.bench_function(
+ &format!("{name}: multiple, {null_percent}% null keys"),
+ |b| {
+ let arrays = vec![
+ nullable_array.clone(),
+ nullable_array.clone(),
+ nullable_array.clone(),
+ ];
+ do_hash_test(b, &arrays);
+ },
+ );
+ }
+ }
Review Comment:
One subtlety with this suggestion: it gives the 50% and 97% cases to every
`DictionaryArray<Int32Type>` in `cases`, including any added later. Please add
a short comment above the `if let` making that intentional, so a future Int32
dictionary benchmark does not pick up the extra densities unexpectedly.
--
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]
