Repository navigation
perf: bound work over dictionary values by the batch's row count - #26703
jayzhan211 wants to merge 8 commits into
Conversation
|
run benchmark clickbench_partitioned |
|
run benchmark with_hashes |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dictionary-work-bounded-by-rows (bc3d609) to 039c0b9 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
baseline:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"
changed:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dictionary-work-bounded-by-rows (bc3d609) to 039c0b9 (merge-base) diff Run configurationrun benchmark with_hashesResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/dictionary-work-bounded-by-rows (bc3d609) to 039c0b9 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
baseline:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"
changed:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/dictionary-work-bounded-by-rows (bc3d609) to 039c0b9 (merge-base) diff Run configurationrun benchmark with_hashesCPU Details (lscpu)Details
Resource Usagewith_hashes — base (merge-base)
with_hashes — branch
File an issue against this benchmark runner |
|
run benchmark clickbench_partitioned |
|
run benchmark with_hashes |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dictionary-work-bounded-by-rows (c459aba) to 039c0b9 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
baseline:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"
changed:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dictionary-work-bounded-by-rows (c459aba) to 039c0b9 (merge-base) diff Run configurationrun benchmark with_hashesResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/dictionary-work-bounded-by-rows (c459aba) to 039c0b9 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
baseline:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"
changed:
env:
DATAFUSION_EXECUTION_PARQUET_ENABLE_RLE_TO_DICTIONARY: "true"CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing perf/dictionary-work-bounded-by-rows (c459aba) to 039c0b9 (merge-base) diff Run configurationrun benchmark with_hashesCPU Details (lscpu)Details
Resource Usagewith_hashes — base (merge-base)
with_hashes — branch
File an issue against this benchmark runner |
…ith a clippy lint
b41cb27 to
eab0efb
Compare
Which issue does this PR close?
Rationale for this change
Batches often carry a dictionary with far more values than rows: each
batch_sizeslice of a GROUP BY's output keeps every group's value, and repartitioning and filtering keep the whole dictionary too. Code that processes every dictionary value then does work proportional to the dictionary, not the batch, so queries slow down as the group count grows.On ClickBench
hits_partitionedwithdatafusion.execution.parquet.enable_rle_to_dictionary = true:takes 13035 ms on main and 1053 ms with this PR:
lowerruns over every group's value once for each 8192-row output slice. Hashing has the same problem: the hash repartition after a partial aggregate hashes every group's value for each slice (#26696).What changes are included in this PR?
Code that processes every value of a dictionary now handles at most
MAX_DICTIONARY_VALUES_PER_ROW(16) values per row. Past that, it first takes the values the rows reference (take(values, keys)with keys0..len), which costs O(rows).datafusion_common::utils::map_dictionary_values(array, f)appliesfto a dictionary's values within that bound. The string functions that keep dictionaries (ascii,btrim,ltrim,rtrim,lower/upper,character_length,initcap,reverse), Sparkhex,array_agg's payload compaction,regexp_replaceandsubstruse it instead of each callingdictionary.with_values(f(values)).regexp_replaceandsubstrkeep their own stricter bounds (regexp_replacedrops unreferenced values whenever there are more values than rows;substrdecodes instead), so their behavior is unchanged.ffails,map_dictionary_valuesruns it once more on only the values the rows reference (garbage_collect_any_dictionary).fcan see values that no row references, for example values behind null keys or left over from a filter, so without the retry it could fail where running it on each row wouldn't. Of the callers above, onlysubstrcan fail because of a particular value (a negative length or a start that overflows).hash_dictionary,hash_dictionary_with_child_hashing) applies the same bound.clippy.tomldisallows callingwith_valuesdirectly, so new code goes through the helper.get_fieldkeeps a direct call because it only picks a child column.The threshold is set by the cheapest per-value work.
lowerandupperon ASCII strings convert the whole values buffer at once, so compacting only pays off for them from about 13 values per row; at 9 values per row it made them 1.3x slower. Just above 16 values per row, compacting is faster than processing every value for each of the string functions above (0.76x forlower/upper, 0.44x forascii, 0.08x to 0.14x for the trims,initcapandreverse) and for hashing (0.31x for short strings andInt64, 0.14x for 64-byte strings). At or below the threshold the code path is unchanged.#24588 proposes evaluating opt-in functions once per distinct value in
ScalarFunctionExprand keeps the per-function dictionary arms. This PR bounds the cost of those arms and of hashing, so the two don't conflict.substr's dictionary fast path (#25874) had its own fallback for a value that no row references makingsubstrfail (a negative length on a batch whose rows are all NULL). The helper's retry now covers that case, so the fallback is removed.Benchmarks
ClickBench
hits_partitioned,enable_rle_to_dictionary = true, 12 partitions,release-nonlto, minimum over 3 to 6 interleaved rounds (ms), same results on both:lowerover a dictionary GROUP BY (the query above)"SearchPhrase"(#26696)arrow_cast("SearchPhrase", 'Utf8')MAX(lower("SearchPhrase"))"MobilePhoneModel"(low cardinality)MAX(lower("MobilePhoneModel"))The rest of the #26696 gap (872 ms vs 494 ms for
Utf8keys) is in arrow'sconcat_dictionaries, which the repartition coalescer uses (apache/arrow-rs#11885), and in the general cost of dictionary group keys (#24822).with_hashes, 8192-row slices of a dictionary with one value per row of the full array, median of 5 interleaved runs (µs). The committeddictionary_slicedcase covers 8 and 32 values per row; the per-type threshold cases were run locally:Int64, just over 16 values per rowInt64, 32 values per rowString functions on an 8192-row slice, both paths timed in one binary (ratio of this PR to main):
lower/uppercharacter_lengthasciibtrim/ltrim/rtriminitcap/reverseThe existing
dictionary_utf8_int32cases (100 values per batch) take the unchanged path and stay within noise.Benchmark bot (12 vCPU), at the current head:
clickbench_partitionedwithenable_rle_to_dictionary = trueon both sides: the bot counts 12 queries as faster. Eleven are 1.26x to 1.91x faster (Q5, Q12–Q14, Q16–Q18, Q33, Q34, Q36, Q39), all grouping onSearchPhraseorURL(Q5 throughCOUNT(DISTINCT "SearchPhrase")); the twelfth is Q32 at 1.06x, which groups on integer columns this PR doesn't touch. The total goes from 74.0 s to 53.0 s. No query is slower by minimum. An earlier run with an 8-values threshold gave the same picture (72.8 s to 52.4 s).dfbench clickbench --query N, 3 runs per side): Q33 peaks about 1.1 GiB higher, and Q16–Q18 and Q34 0.3–0.8 GiB higher. The extra memory is batches waiting inRepartitionExec's output channels. At Q33's peak the repartition reserves 10.3–10.8 GB vs 6.2–6.7 GB on main, while the final aggregate holds 7.5–7.9 GB in both. Hashing no longer slows the senders down, so they get further ahead of the final aggregation. These batches count against the memory pool, so under a memory limit the repartition spills them instead of growing.with_hashes: thedictionary_utf8_int32cases are flat, and the newdictionary_slicedcase takes 53.2 µs at 32 values per row vs 122.5 µs at 8 (the unchanged path). Some cases this PR doesn't touch move by up to 40% in both directions. In both runs,utf8without nulls is 1.24x–1.40x faster,struct_array1.15x–1.19x faster, andlarge_utf8: single, no nulls1.35x–1.38x slower; the smaller moves differ between runs. Locally they are within 0.96x to 1.01x.What is the testing strategy for this PR?
dictionary.slt: a GROUP BY withbatch_size = 1makes every output slice carry many more dictionary values than rows. The test compares each function above on the grouped dictionary with the same function on the decoded value, including a NULL group, and checks the result stays a dictionary. It also goes through the hash repartition of the partial aggregate's slices.create_hashes_for_dict_slices_with_more_values_than_rows(single and multi-column hashes of slices with null keys and null values match the unsliced hashes),map_dictionary_values_bounds_values_per_row, andmap_dictionary_values_fails_only_on_referenced_values(a failing value behind null keys or with no key is ignored, with more rows thanInt8keys can index; a referenced failing value still fails without a retry).string/dictionary_utf8.sltalready checks thatsubstr(c, 1, -1)returns NULL on a batch whose rows are all NULL and still errors when a row is referenced; it now goes through the retry.dictionary_slicedcase indatafusion/common/benches/with_hashes.rs.Are there any user-facing changes?
No behavior changes.
datafusion_common::utilsgainsmap_dictionary_valuesandMAX_DICTIONARY_VALUES_PER_ROW.