perf(parquet): cache dictionary key lookups when writing dictionary arrays - #10862
Open
lyang24 wants to merge 1 commit into
Open
perf(parquet): cache dictionary key lookups when writing dictionary arrays#10862lyang24 wants to merge 1 commit into
lyang24 wants to merge 1 commit into
Conversation
Contributor
|
run benchmark arrow-writer |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dict-key-cache (2bd95a9) to cd7c6b8 (merge-base) diff Run configurationrun benchmark arrow-writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow-writer File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: Kubernetes messageFile an issue against this benchmark runner |
Contributor
|
run benchmark arrow-writer |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dict-key-cache (2bd95a9) to cd7c6b8 (merge-base) diff Run configurationrun benchmark arrow-writerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow-writer File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: Kubernetes messageFile an issue against this benchmark runner |
Contributor
|
run benchmark arrow_writer env:
BENCH_FILTER: dict |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dict-key-cache (2bd95a9) to cd7c6b8 (merge-base) diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "dict"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf/dict-key-cache (2bd95a9) to cd7c6b8 (merge-base) diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "dict"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
|
benchmarks look good |
lyang24
force-pushed
the
perf/dict-key-cache
branch
from
August 29, 2026 20:09
2bd95a9 to
ffe86b7
Compare
Contributor
|
run benchmark arrow_writer env: |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/dict-key-cache (ffe86b7) to edd2775 (merge-base) diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "dict"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf/dict-key-cache (ffe86b7) to edd2775 (merge-base) diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "dict"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
N/A — small, self-contained perf fix. Happy to open one if you'd prefer it tracked.
Rationale for this change
When the input column is already a DictionaryArray, the writer throws that structure away before encoding it. downcast_dict_op! hands a TypedDictionaryArray to encode, which only sees T: ArrayAccessor — so values.value(idx) flattens key → value, and DictEncoder::encode interns once per row. That re-derives, for every row, a mapping the input dictionary already carried: an ahash of the value bytes, a hash table probe, and a byte comparison against the dictionary page.
I found this profiling compaction in GreptimeDB, where mito2 stores each SST's primary key as a Dictionary(UInt32, Binary) column. Rows are sorted by primary key, so keys arrive in long runs — and 82% of the time spent encoding that column was in Interner::intern, re-hashing values the dictionary had already deduplicated.
What changes are included in this PR?
downcast_op! takes a second op, and the Dictionary arms dispatch to it. The new encode_dict keeps the keys visible and resolves each one through a small direct-mapped cache from input dictionary key to interned key. A hit is one comparison and no hashing; the slot also stores the value length so the loop maintains variable_length_bytes without touching the dictionary at all.
The cache is a fixed 24 KiB regardless of dictionary size, which is what lets the lookup be unconditional. I tried an exact per-dictionary remap table first and it needed a size heuristic to avoid regressing high-cardinality columns — a fixed cache degrades on its own instead: columns whose keys repeat hit nearly always, columns with no repetition miss and pay one predictable integer compare per row.
Slots are generation-stamped so a new call invalidates the cache without clearing it. That's required, not an optimization — a key only means anything relative to one input dictionary, and consecutive batches can carry different ones.
Duplicate and unused dictionary entries stay correct: duplicates intern to the same key, unused entries are never interned.
Statistics and bloom filter updates moved into a shared helper so the two entry points can't drift.
Scope: only Dictionary(_, Utf8 | LargeUtf8 | Binary | LargeBinary | FixedSizeBinary) inputs. All other byte array types keep the existing path, and if parquet dictionary encoding is off the fallback encoder is used as before.
Are these changes tested?
Existing tests cover the round trips. Added arrow_writer_dictionary_key_cache_handles_collisions_and_rebinding for the two ways a direct-mapped cache can go wrong: keys colliding in the same slot, and a later batch binding the same key to a different value. Removing either the slot tag check or the generation check makes it fail.
arrow_writer benchmarks, 1,048,576 rows per batch. Running two binaries alternately rather than --save-baseline, because this machine drifted ~13% on string_dictionary/zstd over half an hour and the first comparison I did was mostly measuring that:
string_dictionary_low_cardinality_20/default -47% / -42%
string_dictionary_low_cardinality_20/zstd -39% / -45%
string_dictionary_low_cardinality_100/default -36% / -41%
string_dictionary_low_cardinality_400/default -21% .. -31%
string_dictionary/default -3.3% .. -4.9%
string_dictionary/zstd -1.5% .. -2.2%
string_dictionary/bloom_filter -0.5% / +1.9%
string_dictionary is the high-cardinality case, where the cache mostly misses — it's there to show the miss path doesn't cost anything, not as a win.
Are there any user-facing changes?
no