Skip to content

Commit c459aba

Browse files
committed
perf: raise the dictionary compaction threshold to 16 values per row
1 parent bc3d609 commit c459aba

4 files changed

Lines changed: 28 additions & 25 deletions

File tree

‎datafusion/common/benches/with_hashes.rs‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -389,19 +389,6 @@ fn sliced_array_benchmark(c: &mut Criterion) {
389389
);
390390
}
391391

392-
// Sliced DictionaryArray with one value per row of the full array, like
393-
// the group keys an aggregation emits in `batch_size` slices
394-
{
395-
let full_array = distinct_dictionary_array(total_rows);
396-
let sliced = full_array.slice(slice_offset, slice_len);
397-
c.bench_function(
398-
&format!("dictionary_sliced: 1/{ratio} of {total_rows} values"),
399-
|b| {
400-
do_hash_test_with_len(b, std::slice::from_ref(&sliced), slice_len);
401-
},
402-
);
403-
}
404-
405392
// Sliced Dense UnionArray
406393
{
407394
let full_array = dense_union_array(total_rows);
@@ -420,6 +407,22 @@ fn sliced_array_benchmark(c: &mut Criterion) {
420407
);
421408
}
422409
}
410+
411+
// Sliced DictionaryArray with one value per row of the full array, like the
412+
// group keys an aggregation emits in `batch_size` slices. Hashing reduces
413+
// it to the referenced values only above 16 values per row, so 1/8 keeps
414+
// the full dictionary and 1/32 doesn't.
415+
for ratio in [8, 32] {
416+
let total_rows = BATCH_SIZE * ratio;
417+
let full_array = distinct_dictionary_array(total_rows);
418+
let sliced = full_array.slice(BATCH_SIZE * (ratio / 2), BATCH_SIZE);
419+
c.bench_function(
420+
&format!("dictionary_sliced: 1/{ratio} of {total_rows} values"),
421+
|b| {
422+
do_hash_test_with_len(b, std::slice::from_ref(&sliced), BATCH_SIZE);
423+
},
424+
);
425+
}
423426
}
424427

425428
fn do_hash_test_with_len(b: &mut Bencher, arrays: &[ArrayRef], expected_len: usize) {

‎datafusion/common/src/hash_utils.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,19 +1869,19 @@ mod tests {
18691869
use crate::utils::MAX_DICTIONARY_VALUES_PER_ROW;
18701870

18711871
let values: ArrayRef = Arc::new(StringArray::from_iter(
1872-
(0..20).map(|i| (i != 2).then(|| format!("v{i}"))),
1872+
(0..40).map(|i| (i != 2).then(|| format!("v{i}"))),
18731873
));
1874-
let keys = Int8Array::from_iter((0..24).map(|i| (i % 7 != 1).then_some(i % 20)));
1874+
let keys = Int8Array::from_iter((0..48).map(|i| (i % 7 != 1).then_some(i % 40)));
18751875
let dict: ArrayRef = Arc::new(DictionaryArray::try_new(keys, values).unwrap());
1876-
let ints: ArrayRef = Arc::new(Int32Array::from_iter_values(0..24));
1876+
let ints: ArrayRef = Arc::new(Int32Array::from_iter_values(0..48));
18771877
let random_state = RandomState::with_seed(0);
18781878

18791879
let mut single = vec![0; dict.len()];
18801880
create_hashes([&dict], &random_state, &mut single).unwrap();
18811881
let mut multi = vec![0; dict.len()];
18821882
create_hashes([&ints, &dict], &random_state, &mut multi).unwrap();
18831883

1884-
for (offset, len) in [(0, 2), (1, 2), (8, 1), (22, 2)] {
1884+
for (offset, len) in [(0, 2), (1, 2), (8, 1), (46, 2)] {
18851885
let slice = dict.slice(offset, len);
18861886
assert!(
18871887
slice.as_any_dictionary().values().len()

‎datafusion/common/src/utils/mod.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -441,9 +441,9 @@ where
441441

442442
/// The most dictionary values per row that code processing every value of a
443443
/// dictionary should handle. Past this, taking the values the rows reference
444-
/// is cheaper than processing all of them, even for values that are cheap to
445-
/// process such as short strings or integers being hashed.
446-
pub const MAX_DICTIONARY_VALUES_PER_ROW: usize = 8;
444+
/// is cheaper than processing all of them, even for the cheapest per-value
445+
/// work, such as hashing integers or changing the case of ASCII strings.
446+
pub const MAX_DICTIONARY_VALUES_PER_ROW: usize = 16;
447447

448448
/// Returns `None` when `array` has at most [`MAX_DICTIONARY_VALUES_PER_ROW`]
449449
/// values per row, and otherwise a copy whose values are `take(values, keys)`
@@ -2263,13 +2263,13 @@ mod tests {
22632263

22642264
#[test]
22652265
fn map_dictionary_values_bounds_values_per_row() -> Result<()> {
2266-
let strings = (0..12)
2266+
let strings = (0..24)
22672267
.map(|i| (i % 5 != 1).then(|| "x".repeat(i)))
22682268
.collect::<Vec<_>>();
22692269
let dict: DictionaryArray<Int32Type> =
22702270
strings.iter().map(|s| s.as_deref()).collect();
22712271
let dict: ArrayRef = Arc::new(dict);
2272-
for (offset, len) in [(0, 12), (1, 1), (6, 1)] {
2272+
for (offset, len) in [(0, 24), (1, 1), (6, 1)] {
22732273
let slice = dict.slice(offset, len);
22742274
let mut seen = 0;
22752275
let mapped = map_dictionary_values(&slice, |values| {

‎datafusion/sqllogictest/test_files/dictionary.slt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -665,15 +665,15 @@ set datafusion.execution.target_partitions = 2;
665665
statement ok
666666
create table padded_groups as
667667
select arrow_cast(
668-
case when value % 13 = 0 then null else ' Grp' || (value % 40) || ' ' end,
668+
case when value % 13 = 0 then null else ' Grp' || (value % 100) || ' ' end,
669669
'Dictionary(Int32, Utf8)'
670670
) as p
671-
from generate_series(1, 400);
671+
from generate_series(1, 1000);
672672

673673
query I
674674
select count(*) from (select p from padded_groups group by p);
675675
----
676-
41
676+
101
677677

678678
# Each function on the grouped dictionary must match the same function on the
679679
# decoded value

0 commit comments

Comments
 (0)