Skip to content

perf: skip null keys when scattering nullable dictionary hashes - #26966

Open
lakshaydevoo-arch wants to merge 8 commits into
apache:mainfrom
lakshaydevoo-arch:perf/26848-dictionary-hash-scatter
Open

lakshaydevoo-arch wants to merge 8 commits into
apache:mainfrom
lakshaydevoo-arch:perf/26848-dictionary-hash-scatter

Conversation

@lakshaydevoo-arch

@lakshaydevoo-arch lakshaydevoo-arch commented Oct 31, 2026 •

Copy link
Copy Markdown

Important

Fork CI setup: For pull requests from a fork to main, enable workflows in your fork's Actions tab before you push.
See the fork CI setup guide.

Which issue does this PR close?

This covers the nullable-key path only. As discussed with author, I'll send the non-null path as a separate PR.

Rationale for this change

When create_hashes hashes a dictionary array, it hashes each dictionary value once and then scatters those hashes out to the rows through the keys. If the keys have nulls, that scatter loop goes through keys().iter(). It builds an Option for every row and branches on it, so the cost depends on the total number of rows, not the number of valid keys.

The with_hashes benchmarks show this clearly. 97% null keys cost about as much as 3%, even though there's roughly 30x less real work to do. At 50% nulls the branch can't be predicted, so that case was the slowest of all.

What changes are included in this PR?

  • In the null-keys branch of hash_dictionary_scatter, the loop now goes over NullBuffer::valid_indices() instead of keys().iter(). Only valid key slots are visited, so:
    • null slots keep their existing hash, as before;
    • the keys stored behind null slots are never read. Arrow allows those to be any value, including out of range;
    • null dictionary values, the multi-column combine_hashes path and the existing behaviour of stopping at hashes_buffer.len() are all unchanged.
  • The non-null branch is untouched.
  • No new unsafe. The existing get_unchecked on the dictionary hashes is now only reached from valid key slots.
  • Two new with_hashes benchmark densities: Int32 dictionaries with 50% and 97% null keys, single and multiple columns. 0% and 3% were already covered by the existing dictionary_utf8_int32 cases. The existing seeded null mask still produces exactly the same output.

I also tried handling validity in 64-row words, with a dense loop for fully-valid words, as the issue suggests. On these benchmarks it was within ±4–9% of the simpler version, in both directions, so I left it out to keep the change small. I also checked whether key width matters (Int8 / Int32 / UInt64) and couldn't find a consistent difference, so I didn't add key-width benchmarks.

What is the testing strategy for this PR?

Added create_hashes_for_dict_arrays_with_null_keys in hash_utils.rs. It compares dictionary hashes with the hashes of the equivalent StringArray across:

  • Int8, Int32 and Int64 keys;
  • dictionary values with and without a null value;
  • lengths 0, 1, 7, 8, 9, 63, 64, 65, 127, 128 and 129, at slice offsets 0, 3, 5 and 64 (covering offsets that aren't byte-aligned);
  • out-of-range keys (-1, values.len(), 127) hidden behind null slots;
  • one column and two columns (an Int32 column first, so "unchanged" can be told apart from "zero");
  • a hashes_buffer shorter than the array;
  • an empty values array with all keys null.

The test passes against the old loop too, so behaviour is unchanged. It fails if the loop is broken or loses the truncation.

Checked locally:

  • cargo fmt --all -- --check
  • cargo clippy --all-targets --workspace --features avro,integration-tests,extended_tests -- -D warnings
  • cargo test -p datafusion-common, both with and without force_hash_collisions

Benchmarks

cargo bench -p datafusion-common --bench with_hashes -- dictionary, 3 runs on each side, median. Intel i5-12500H (WSL2). From what I measured, anything under about 6% is run-to-run noise on this machine.

benchmark main this PR change
single, no nulls 2.90 µs 3.00 µs noise (code unchanged)
multiple, no nulls 9.29 µs 9.53 µs noise (code unchanged)
single, 3% null keys 7.60 µs 6.04 µs -21%
multiple, 3% null keys 22.95 µs 19.29 µs -16%
single, 50% null keys 12.97 µs 3.58 µs -72%
multiple, 50% null keys 73.57 µs 10.92 µs -85%
single, 97% null keys 7.22 µs 1.35 µs -81%
multiple, 97% null keys 20.20 µs 2.51 µs -88%

A few notes:

  • main's multiple / 50% number was oddly slow, about 2x the same case with Int8 keys, and I couldn't work out why. So part of that -85% is that slowdown going away, not just this change.
  • 3% nulls is still around 2x the no-null case. There's probably more to gain there, but I didn't want to add complexity without numbers that clearly justify it.
  • These are all from one x86 machine. An ARM run from the benchmark bot would be great if someone can trigger it.

Are there any user-facing changes?

No. This is an internal performance change with no API changes.

@lakshaydevoo-arch

Copy link
Copy Markdown
Author

@Rich-T-kid see the nullable path optimizations are feasible or any more requirements from your side.

@lakshaydevoo-arch

Copy link
Copy Markdown
Author

@jayzhan211 asking for a review sir

@Rich-T-kid

Copy link
Copy Markdown
Contributor

👋 sorry for the delay Ill try and take a look at this

@Rich-T-kid

Copy link
Copy Markdown
Contributor

run benchmark with_hashes
env:
BENCH_FILTER: dictionary_utf8_int32

@adriangbot

Copy link
Copy Markdown

🤖 Benchmark running (GKE) | trigger
Instance: c4a-highmem-16 (12 vCPU / 65 GiB) | Linux bench-c6569220435-4221-k45wt 6.12.105+ #1 SMP Sat Sep 19 07:55:25 UTC 2026 aarch64 GNU/Linux

CPU Details (lscpu)
Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected

Comparing perf/26848-dictionary-hash-scatter (aa745d4) to 5db4b23 (merge-base) diff

Run configuration
run benchmark with_hashes
env:
  BENCH_FILTER: "dictionary_utf8_int32"

Results will be posted here when complete


File an issue against this benchmark runner

@adriangbot

Copy link
Copy Markdown

🤖 Benchmark completed (GKE) | trigger

Instance: c4a-highmem-16 (12 vCPU / 65 GiB)

Comparing perf/26848-dictionary-hash-scatter (aa745d4) to 5db4b23 (merge-base) diff

Run configuration
run benchmark with_hashes
env:
  BENCH_FILTER: "dictionary_utf8_int32"
CPU Details (lscpu)
Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected
Details

group                                             HEAD                                   perf_26848-dictionary-hash-scatter
-----                                             ----                                   ----------------------------------
dictionary_utf8_int32: multiple, 50% null keys                                           1.00     12.9±0.02µs        ? ?/sec
dictionary_utf8_int32: multiple, 97% null keys                                           1.00      2.6±0.01µs        ? ?/sec
dictionary_utf8_int32: multiple, no nulls         1.00     10.8±0.02µs        ? ?/sec    1.00     10.8±0.01µs        ? ?/sec
dictionary_utf8_int32: multiple, nulls            1.13     26.4±0.02µs        ? ?/sec    1.00     23.3±0.02µs        ? ?/sec
dictionary_utf8_int32: single, 50% null keys                                             1.00      4.2±0.01µs        ? ?/sec
dictionary_utf8_int32: single, 97% null keys                                             1.00   1156.7±4.74ns        ? ?/sec
dictionary_utf8_int32: single, no nulls           1.00      3.2±0.00µs        ? ?/sec    1.00      3.2±0.00µs        ? ?/sec
dictionary_utf8_int32: single, nulls              1.09      8.1±0.01µs        ? ?/sec    1.00      7.5±0.01µs        ? ?/sec

Resource Usage

with_hashes — base (merge-base)

Metric Value
Wall time 110.0s
Peak memory 32.4 MiB
Avg memory 9.2 MiB
CPU user 47.0s
CPU sys 0.0s
Peak spill 0 B

with_hashes — branch

Metric Value
Wall time 140.0s
Peak memory 42.2 MiB
Avg memory 19.0 MiB
CPU user 94.2s
CPU sys 0.1s
Peak spill 0 B

File an issue against this benchmark runner

@Rich-T-kid Rich-T-kid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lakshaydevoo-arch can you split benchmarks into a separate PR so we can use the bot to compare benchmarks

@lakshaydevoo-arch

Copy link
Copy Markdown
Author

thanks for pointing that out i will move the benchmarks addition to seprate PR so bot can review.

@lakshaydevoo-arch

Copy link
Copy Markdown
Author

I've separated the additional benchmarks into a dedicated PR (#27008 ) and removed them from this optimization PR.

This PR now focuses on the nullable dictionary hash scattering optimization and its regression tests. The benchmark PR can be merged first so the bot can compare the same cases against the baseline.

@lakshaydevoo-arch

Copy link
Copy Markdown
Author

@Rich-T-kid you may look at the new PR #27008 i made

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants