Repository navigation
perf(parquet): Skip per-level validity pass for list children with no nulls - #11376
Merged
alamb merged 6 commits intoOct 7, 2026
Merged
Conversation
The `ListArray` benchmarks pick list lengths from `0..MAX_LIST_LEN`, so even the "no NULLs" cases have about 10% empty lists, and every batch contains definition levels below the maximum. Columns of lists that are never null or empty and have no null elements are common, but none of the benchmarks cover them. Add an `Int32List` case with no nulls and lengths from 1 to `MAX_LIST_LEN - 1`, so that every definition level is at the maximum.
…out nulls Since apache#9848, reading the child column of a list builds a compact validity bitmap with a pass over every definition level, even when none of them is null. When every definition level in a chunk is at the maximum (`values_to_read == levels_to_read`), each level is a non-null child value, so append an all-valid run instead. This pass caused the list decoding regression between 58.3.0 and 59.1.0. Reading a list<int64> column (5M rows, 62.5M values, written by pyarrow) with `ParquetRecordBatchReader` on an Apple M4 Max: 289 ms -> 231 ms, back to 58.3.0's 232 ms. The `arrow_array_reader/ListArray/Int32List/no NULLs or empty lists` benchmark is 18-20% faster on the same machine.
Contributor
Author
|
FYI @HippoBaro @alamb |
Contributor
|
run benchmark arrow_reader |
|
🤖 Benchmark starting (GKE) | trigger Target: arrow_reader Sharding factor: 1 (1 workers). Comparing neilc/fix-parquet-list-validity-regression (fa6d271) to e97b1aa (merge-base) diff Run configurationrun benchmark arrow_reader
shards: 1
Results will be posted when all workers finish. File an issue against this benchmark runner |
Contributor
|
Hi @neilconway let me take a look! |
HippoBaro
approved these changes
Oct 4, 2026
|
🤖 Benchmark completed (GKE) | trigger Target: arrow_reader Comparing neilc/fix-parquet-list-validity-regression (fa6d271) to e97b1aa (merge-base) diff Run configurationrun benchmark arrow_reader
shards: 1
DetailsPer-runner informationarrow_reader — shard 1/1Node: gk3-benchmark-cluster-nap-r1moxof8-92912e2d-p65f Instance: c4a-highmem-16 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_readerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
…t-validity-regression
Contributor
Thanks @HippoBaro -- can you please make a PR with that test? |
Contributor
|
I merged up from main so we can get bot confirmation of the performance improvement |
Contributor
|
run benchmark arrow_reader env: |
|
🤖 Benchmark starting (GKE) | trigger Target: arrow_reader Sharding factor: 1 (1 workers). Comparing neilc/fix-parquet-list-validity-regression (c8eba8c) to 103ebe7 (merge-base) diff Run configurationrun benchmark arrow_reader
env:
BENCH_FILTER: ListArray/Int32List
shards: 1
Results will be posted when all workers finish. File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Target: arrow_reader Comparing neilc/fix-parquet-list-validity-regression (c8eba8c) to 103ebe7 (merge-base) diff Run configurationrun benchmark arrow_reader
env:
BENCH_FILTER: ListArray/Int32List
shards: 1
DetailsPer-runner informationarrow_reader — shard 1/1Node: gk3-benchmark-cluster-nap-4hvbnhcy-8466d65f-nr44 Instance: c4a-highcpu-48 (12 vCPU / 65 GiB) uname: BENCH_COMMAND: cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_readerCPU Details (lscpu)Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
neilconway
force-pushed
the
neilc/fix-parquet-list-validity-regression
branch
from
October 5, 2026 18:06
c8eba8c to
584b96e
Compare
Contributor
Author
|
@alamb @HippoBaro I took the liberty of adding the test that @HippoBaro suggested to this PR; let me know if you think it looks reasonable. @alamb Looks like the benchmark bot results are what I'd expect for this optimization. |
Contributor
|
Looks great -- thank you @neilconway and @HippoBaro |
alamb
enabled auto-merge
October 5, 2026 23:22
Contributor
|
@alamb did you mean to approve & enable auto merge? 😅 |
Contributor
Yes, sory about that |
alamb
approved these changes
Oct 7, 2026
Contributor
There was a problem hiding this comment.
Thanks @neilconway and @HippoBaro -- the code looks fine to me
Contributor
|
Sorry forgot to click submit |
Contributor
Author
|
@alamb The perf figure I mentioned didn't come from a real use-case. It comes from some local benchmark scripts I use to track UDF performance in DataFusion, and those store their data in Parquet. |
Contributor
Author
|
Thanks for the reviews! @alamb @HippoBaro |
Rich-T-kid
pushed a commit
to Rich-T-kid/arrow-rs
that referenced
this pull request
Oct 7, 2026
… nulls (apache#11376) # Which issue does this PR close? - N/A # Rationale for this change Since apache#9848, reading the child column of a list builds a compact validity bitmap with a pass over every definition level. When every definition level in a chunk has the maximum value (i.e., there are no null lists, empty lists, or null elements in any list), we can skip doing the pass over the definition levels and append an all-valid run instead. Adding this pass caused a regression in list decoding performance between 58.3.0 and 59.1.0: for example, 232 ms -> 289 ms to read 5M rows with 62.5M values on an Apple M4 Max. Benchmark for this case was added in apache#11375. Results on M4 Max: - StringList/no NULLs: +0.7%, −0.6%, +0.9% - StringList/half NULLs: +2.8%, +3.8%, +1.0% - StringList/90pct NULLs: +2.7%, +0.5%, +0.9% - StringList/99pct NULLs: +1.0%, +0.2%, +0.7% - Int32List/no NULLs: +0.8%, −0.3%, −0.2% - Int32List/half NULLs: +1.3%, −0.3%, +0.5% - Int32List/90pct NULLs: +0.6%, −0.9%, −0.3% - Int32List/99pct NULLs: +1.1%, −0.6%, +1.0% - Int32List/no NULLs or empty lists: −18.4%, −19.9%, −18.5% - Fixed32List/no NULLs: +1.2%, −0.8%, +1.1% - Fixed32List/half NULLs: +1.6%, +0.8%, +2.1% - Fixed32List/90pct NULLs: +0.4%, −0.5%, −0.5% - Fixed32List/99pct NULLs: +0.2%, +0.4%, +0.9% # What changes are included in this PR? See above. # Are these changes tested? Covered by existing tests. # Are there any significant user-facing changes? No. # AI usage Developed and revised with Claude Code (Opus 5.5).
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
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?
Rationale for this change
Since #9848, reading the child column of a list builds a compact validity bitmap with a pass over every definition level. When every definition level in a chunk has the maximum value (i.e., there are no null lists, empty lists, or null elements in any list), we can skip doing the pass over the definition levels and append an all-valid run instead.
Adding this pass caused a regression in list decoding performance between 58.3.0 and 59.1.0: for example, 232 ms -> 289 ms to read 5M rows with 62.5M values on an Apple M4 Max.
Benchmark for this case was added in #11375. Results on M4 Max:
What changes are included in this PR?
See above.
Are these changes tested?
Covered by existing tests.
Are there any significant user-facing changes?
No.
AI usage
Developed and revised with Claude Code (Opus 5.5).