Skip to content

Avoid expanding unreferenced struct plan fields - #9254

Draft
joseph-isaacs wants to merge 1 commit into
vortex-plan-zoned-pruningfrom
ji/vx-plan-perf
Draft

Avoid expanding unreferenced struct plan fields#9254
joseph-isaacs wants to merge 1 commit into
vortex-plan-zoned-pruningfrom
ji/vx-plan-perf

Conversation

@joseph-isaacs

Copy link
Copy Markdown
Contributor

Summary

  • keep direct bound GetItem(root) expressions intact when partitioning a struct plan
  • expand Select(root) into only the selected fields
  • expand every field only when an expression genuinely consumes the bare struct root
  • preserve include/exclude projection semantics with focused plan snapshots

Root cause

ExpressionStructRule eagerly built a bound pack containing every struct field before it knew
which fields the query referenced. On the 105-field ClickBench schema, every rule application
constructed 105 GetItem expressions. Binding each node resolved its return dtype, repeatedly
deserializing lazy field dtypes even for a one-column query.

The expression is already bound, and the bound field annotator can partition direct field access
without expanding it. This change only synthesizes field accesses where Select or a bare root
semantically requires them.

Performance

The benchmark opens all 100 ClickBench Vortex shards, filters AdvEngineID != 0, projects
AdvEngineID, warms planning once, and reports the median of 25 complete planning passes.

Stack Planning Prepared execution Rows
pre-BoundExpression stack 8.28 ms 46.14 ms 630,500
#9244 with bound plans 7.36 ms 45.91 ms 630,500
this PR 2.46 ms 46.46 ms 630,500

This is a 66.6% planning reduction from the bound baseline and a 70.3% reduction from the original
stack. Execution is unchanged within run-to-run noise.

A matched Samply planning-only profile over 500 all-file passes reduced main-thread CPU from
8.22 s to 1.23 s. The previous hot stack through BoundExpression::try_new,
GetItem::return_dtype, and lazy dtype deserialization is absent after the change.

Stack

This draft is stacked on #9244 and contains one signed commit.

Validation

  • RUSTC_WRAPPER= cargo nextest run -p vortex-layout -p vortex-scan-v2 — 215 passed
  • RUSTC_WRAPPER= cargo clippy --all-targets --all-features -- -D warnings
  • RUSTC_WRAPPER= cargo test --doc -p vortex-layout -p vortex-scan-v2
  • cargo +nightly fmt --all -- --check
  • git diff --check

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
@codspeed-hq

codspeed-hq Bot commented Aug 7, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 83.01%

⚡ 1 improved benchmark
✅ 1924 untouched benchmarks
⏩ 51 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation decompress[u64, (1000, 16)] 133.6 µs 73 µs +83.01%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ji/vx-plan-perf (6f8023d) with vortex-plan-zoned-pruning (1bdf135)

Open in CodSpeed

Footnotes

  1. 51 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.

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

Labels

changelog/performance A performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant