From 9b2ee2ec41b08b86c7cba4f68513f01c499a96da Mon Sep 17 00:00:00 2001 From: Lakshya Sharma <229351168+lakshaydevoo-arch@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:26:21 +0000 Subject: [PATCH 1/5] bench: add nullable dictionary hash density benchmarks --- datafusion/common/benches/with_hashes.rs | 60 +++++++++++++++++++++--- 1 file changed, 54 insertions(+), 6 deletions(-) diff --git a/datafusion/common/benches/with_hashes.rs b/datafusion/common/benches/with_hashes.rs index 7fc4a29fc55ec..3a22325e58781 100644 --- a/datafusion/common/benches/with_hashes.rs +++ b/datafusion/common/benches/with_hashes.rs @@ -178,8 +178,13 @@ fn create_null_mask(len: usize) -> NullBuffer where StandardUniform: Distribution, { + create_null_mask_with_density(len, 0.03).expect("should be nulls in buffer") +} + +/// Returns a random null mask where each slot is null with probability +/// `null_density`, or `None` if no slot ended up null +fn create_null_mask_with_density(len: usize, null_density: f32) -> Option { let mut rng = make_rng(); - let null_density = 0.03; let mut builder = NullBufferBuilder::new(len); for _ in 0..len { if rng.random::() < null_density { @@ -188,7 +193,21 @@ where builder.append_non_null(); } } - builder.finish().expect("should be nulls in buffer") + builder.finish() +} + +/// Returns `dict` with a new random null mask on its keys, where each key is null +/// with probability `null_density`. Any existing key nulls are replaced, and the +/// key and value buffers are shared rather than copied +fn with_null_keys( + dict: &DictionaryArray, + null_density: f32, +) -> ArrayRef { + let keys = PrimitiveArray::::new( + dict.keys().values().clone(), + create_null_mask_with_density(dict.len(), null_density), + ); + Arc::new(DictionaryArray::new(keys, Arc::clone(dict.values()))) } // Returns a new array that is the same as array, but with nulls @@ -290,9 +309,15 @@ impl StringPool { &self, array_length: usize, ) -> ArrayRef { - Arc::new(DictionaryArray::::from_iter( - self.iter_strings(array_length), - )) + Arc::new(self.typed_dictionary_array::(array_length)) + } + + /// Like [`Self::dictionary_array`], but returns the concrete `DictionaryArray` + fn typed_dictionary_array( + &self, + array_length: usize, + ) -> DictionaryArray { + DictionaryArray::::from_iter(self.iter_strings(array_length)) } } @@ -645,10 +670,33 @@ fn mixed_columns_benchmark(c: &mut Criterion) { }); } +/// Int32 dictionaries with 50% and 97% null keys. 0% and 3% are covered by +/// the existing `dictionary_utf8_int32` "no nulls" and "nulls" cases in +/// `criterion_benchmark` +fn dictionary_benchmark(c: &mut Criterion) { + let pool = StringPool::new(100, 64); + let dict = pool.typed_dictionary_array::(BATCH_SIZE); + let name = "dictionary_utf8_int32"; + for null_percent in [50, 97] { + let 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(&array)); + }); + c.bench_function( + &format!("{name}: multiple, {null_percent}% null keys"), + |b| { + let arrays = vec![array.clone(), array.clone(), array.clone()]; + do_hash_test(b, &arrays); + }, + ); + } +} + criterion_group!( benches, criterion_benchmark, sliced_array_benchmark, - mixed_columns_benchmark + mixed_columns_benchmark, + dictionary_benchmark ); criterion_main!(benches); From 5cb5ed5e39fa8de3d55c8de77b92883453d8b1f9 Mon Sep 17 00:00:00 2001 From: Lakshya Sharma <229351168+lakshaydevoo-arch@users.noreply.github.com> Date: Tue, 3 Nov 2026 08:02:48 +0000 Subject: [PATCH 2/5] refactor: integrate null-density cases into existing benchmarks --- datafusion/common/benches/with_hashes.rs | 64 ++++++++++++------------ 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/datafusion/common/benches/with_hashes.rs b/datafusion/common/benches/with_hashes.rs index 3a22325e58781..2f8e844fca075 100644 --- a/datafusion/common/benches/with_hashes.rs +++ b/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::>() + .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); + }, + ); + } + } } } @@ -309,15 +336,9 @@ impl StringPool { &self, array_length: usize, ) -> ArrayRef { - Arc::new(self.typed_dictionary_array::(array_length)) - } - - /// Like [`Self::dictionary_array`], but returns the concrete `DictionaryArray` - fn typed_dictionary_array( - &self, - array_length: usize, - ) -> DictionaryArray { - DictionaryArray::::from_iter(self.iter_strings(array_length)) + Arc::new(DictionaryArray::::from_iter( + self.iter_strings(array_length), + )) } } @@ -670,33 +691,10 @@ fn mixed_columns_benchmark(c: &mut Criterion) { }); } -/// Int32 dictionaries with 50% and 97% null keys. 0% and 3% are covered by -/// the existing `dictionary_utf8_int32` "no nulls" and "nulls" cases in -/// `criterion_benchmark` -fn dictionary_benchmark(c: &mut Criterion) { - let pool = StringPool::new(100, 64); - let dict = pool.typed_dictionary_array::(BATCH_SIZE); - let name = "dictionary_utf8_int32"; - for null_percent in [50, 97] { - let 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(&array)); - }); - c.bench_function( - &format!("{name}: multiple, {null_percent}% null keys"), - |b| { - let arrays = vec![array.clone(), array.clone(), array.clone()]; - do_hash_test(b, &arrays); - }, - ); - } -} - criterion_group!( benches, criterion_benchmark, sliced_array_benchmark, - mixed_columns_benchmark, - dictionary_benchmark + mixed_columns_benchmark ); criterion_main!(benches); From 190f145a9b1c502448fc7d03c97861fe133833de Mon Sep 17 00:00:00 2001 From: Lakshya Sharma <229351168+lakshaydevoo-arch@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:52:12 +0530 Subject: [PATCH 3/5] Update datafusion/common/benches/with_hashes.rs Co-authored-by: RIchard Baah <137434454+Rich-T-kid@users.noreply.github.com> --- datafusion/common/benches/with_hashes.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/datafusion/common/benches/with_hashes.rs b/datafusion/common/benches/with_hashes.rs index 2f8e844fca075..470205197a50b 100644 --- a/datafusion/common/benches/with_hashes.rs +++ b/datafusion/common/benches/with_hashes.rs @@ -161,18 +161,19 @@ fn criterion_benchmark(c: &mut Criterion) { }); } - if name == "dictionary_utf8_int32" { - let dict = array - .as_any() - .downcast_ref::>() - .expect("Expected Int32 dictionary"); - + if let Some(dict) = array + .as_any() + .downcast_ref::>() + { for null_percent in [50, 97] { - let nullable_array = with_null_keys(dict, null_percent as f32 / 100.0); + 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)), + |b| { + do_hash_test(b, std::slice::from_ref(&nullable_array)); + }, ); c.bench_function( &format!("{name}: multiple, {null_percent}% null keys"), From 0792ce820145f7d9424fd3726460611d461fd549 Mon Sep 17 00:00:00 2001 From: Lakshya Sharma <229351168+lakshaydevoo-arch@users.noreply.github.com> Date: Tue, 3 Nov 2026 19:56:26 +0530 Subject: [PATCH 4/5] Enhance comments in with_hashes.rs for clarity Add comments for future null-density cases in benchmarks. --- datafusion/common/benches/with_hashes.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datafusion/common/benches/with_hashes.rs b/datafusion/common/benches/with_hashes.rs index 470205197a50b..f7a03c931b274 100644 --- a/datafusion/common/benches/with_hashes.rs +++ b/datafusion/common/benches/with_hashes.rs @@ -160,7 +160,8 @@ fn criterion_benchmark(c: &mut Criterion) { do_hash_test(b, &arrays); }); } - + // Apply additional null-density cases to all Int32 dictionary + // benchmarks, including any added in the future. if let Some(dict) = array .as_any() .downcast_ref::>() From d4f90cdd4596904a8062bd70880044a458f102b9 Mon Sep 17 00:00:00 2001 From: Lakshya Sharma <229351168+lakshaydevoo-arch@users.noreply.github.com> Date: Tue, 3 Nov 2026 15:02:38 +0000 Subject: [PATCH 5/5] style: format dictionary benchmark changes --- datafusion/common/benches/with_hashes.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/datafusion/common/benches/with_hashes.rs b/datafusion/common/benches/with_hashes.rs index f7a03c931b274..c4fda45e3e204 100644 --- a/datafusion/common/benches/with_hashes.rs +++ b/datafusion/common/benches/with_hashes.rs @@ -162,13 +162,9 @@ fn criterion_benchmark(c: &mut Criterion) { } // Apply additional null-density cases to all Int32 dictionary // benchmarks, including any added in the future. - if let Some(dict) = array - .as_any() - .downcast_ref::>() - { + if let Some(dict) = array.as_any().downcast_ref::>() { for null_percent in [50, 97] { - let nullable_array = - with_null_keys(dict, null_percent as f32 / 100.0); + let nullable_array = with_null_keys(dict, null_percent as f32 / 100.0); c.bench_function( &format!("{name}: single, {null_percent}% null keys"),