Skip to content

Lazily untranspose DeltaArray validity and depend on fastlanes bit transpose code - #8725

Merged
robert3005 merged 6 commits into
developfrom
rk/transposedvalidity
Jul 21, 2026
Merged

Lazily untranspose DeltaArray validity and depend on fastlanes bit transpose code#8725
robert3005 merged 6 commits into
developfrom
rk/transposedvalidity

Conversation

@robert3005

Copy link
Copy Markdown
Contributor

Delta array unnecessarily untraposes whole validity whenever it's queried. This
pr avoids this by laziliy wrapping it in a TransposedBool array that does
untranspose when necessary.

While we're at it we remove the bit transpose logic that has been moved to
fastlanes repo

@robert3005 robert3005 added the changelog/feature A new feature label Jul 12, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 12, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 1788 untouched benchmarks
⏩ 54 skipped benchmarks1


Comparing rk/transposedvalidity (ac06a40) with develop (1113b83)

Open in CodSpeed

Footnotes

  1. 54 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@robert3005
robert3005 force-pushed the rk/transposedvalidity branch from 26bff75 to 0fd420f Compare July 12, 2026 16:15
Comment thread encodings/fastlanes/benches/bit_transpose.rs Outdated
Comment thread encodings/fastlanes/benches/bit_transpose.rs Outdated
Comment thread encodings/fastlanes/src/bit_transpose/validity.rs Outdated
Comment thread encodings/fastlanes/src/bit_transpose/validity.rs Outdated
Comment on lines +143 to +146
struct TransposedBoolMetadata {
#[prost(uint32, tag = "1")]
offset: u32,
}

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.

The transport is width dependent no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what you mean

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have removed serialisation from this array. We can probably refactor it to share logic with bitpacked/delta arrays

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.

There is a lot of shared logic with out blocked arrays?

@robert3005
robert3005 force-pushed the rk/transposedvalidity branch 2 times, most recently from 774f405 to d830e48 Compare July 13, 2026 14:57
…anspose code

Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
robert3005 force-pushed the rk/transposedvalidity branch from dd0e1d9 to e7ae6b8 Compare July 21, 2026 13:15
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done ac06a40 1 Explore Profiling Data
Previous Runs (1)
Status Commit Job Attempt Link
🟢 Done e7ae6b8 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Benchmarks: Vortex queries 📖

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +6.6%
Engines: DataFusion No clear signal (+3.7%, environment too noisy confidence) · DuckDB No clear signal (+9.5%, medium confidence)
Vortex (geomean): 1.179x ❌
Parquet (geomean): 1.142x ❌
Shifts: Parquet (control) +14.2% · Median polish +19.5%

How to read Verdict and Engines
  • Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
  • Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
  • Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.

datafusion / vortex-file-compressed (1.243x ❌, 0↑ 2↓)
name PR ac06a40 (ns) base 1113b83 (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 🚨 12626363 9897812 1.28
vortex_q01/datafusion:vortex-file-compressed 🚨 7692533 6349778 1.21
datafusion / parquet (1.199x ❌, 0↑ 2↓)
name PR ac06a40 (ns) base 1113b83 (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 🚨 25356891 21935536 1.16
vortex_q01/datafusion:parquet 🚨 6423019 5164883 1.24
duckdb / vortex-file-compressed (1.191x ❌, 0↑ 2↓)
name PR ac06a40 (ns) base 1113b83 (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 🚨 12547030 10659218 1.18
vortex_q01/duckdb:vortex-file-compressed 🚨 7803701 6480145 1.20
duckdb / parquet (1.087x ➖, 0↑ 1↓)
name PR ac06a40 (ns) base 1113b83 (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 🚨 25938423 23522821 1.10
vortex_q01/duckdb:parquet 10151304 9472792 1.07

No file size changes detected.

Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005
robert3005 enabled auto-merge (squash) July 21, 2026 13:44
@robert3005
robert3005 merged commit 94e593e into develop Jul 21, 2026
90 of 91 checks passed
@robert3005
robert3005 deleted the rk/transposedvalidity branch July 21, 2026 13:53
robert3005 pushed a commit that referenced this pull request Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants