Skip to content

Thread Scope through the optimizer, coercion and partitioning - #9338

Draft
mhk197 wants to merge 1 commit into
mk/lambda-variablesfrom
mk/scoped-passes
Draft

Thread Scope through the optimizer, coercion and partitioning#9338
mhk197 wants to merge 1 commit into
mk/lambda-variablesfrom
mk/scoped-passes

Conversation

@mhk197

@mhk197 mhk197 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Stacked on #9257.

Why

optimize, coerce_expression and partition all took a &DType and typed every node against it. That works for a flat tree but has two consequences:

  • a Variable is untypeable, so each pass grew its own lambda bail-out
  • the failure mode for a future pass is to descend blindly and mistype a body against the root dtype

What

All three now take impl Into<Scope>, and Expression::return_dtype resolves Root and Variable against that scope instead of bailing on the latter. DType: Into<Scope> yields a scope binding no frames, so existing callers pass a root dtype unchanged and still resolve no variables — the fail-closed property is kept, but it is now a consequence of the scope being empty rather than a special case.

A caller that does supply a frame can now resolve variables through these passes. That is what struct-expression partitioning needs in order to use real variables instead of encoding its environment as a synthetic struct.

SimplifyCache is gone

Its typing logic reduced to expr.return_dtype(&scope) once return_dtype became scope-aware, leaving only the memo. That memo was keyed on Expression, whose Hash walks the entire subtree — so each lookup paid the same O(subtree) cost it was meant to save.

Measured on expr_optimize (or-chain of 200, 4 runs each, medians):

median range
with memo 46.1 – 53.3 µs
without 44.8 – 48.3 µs

Removing it is the same or slightly faster. It was also a latent correctness hazard: keyed on the expression alone, it goes unsound the moment the scope varies mid-walk.

Lambdas are still a boundary

Not an oversight. A lambda's parameter dtypes come from whoever applies it, and an unbound tree records no applier, so there is no frame to push. A higher-order function knows those types; at that point each bail-out becomes a push_frame. Until then, coercion and constant-folding do not run inside a lambda body — conservative, and currently unobservable since nothing constructs an applied lambda.

Checks

  • cargo build --workspace
  • cargo nextest run -p vortex-array — 3283 passed
  • cargo nextest run -p vortex-layout -p vortex-file -p vortex-scan -p vortex-datafusion -p vortex-geo -p vortex-btrblocks — 848 passed
  • cargo test --doc -p vortex-array — 73 passed
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --workspace
  • cargo clippy --all-targets --all-features — clean
  • cargo +nightly fmt --all, git diff --check
  • cargo bench -p vortex-array --bench expr_optimize — no regression

New tests cover a variable resolving through a frame in both optimize and coerce, and being rejected without one.

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@codspeed-hq

codspeed-hq Bot commented Aug 10, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 17.52%

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

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation fsl_large 239.2 µs 203.5 µs +17.52%

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 mk/scoped-passes (8d7300f) with mk/lambda-variables (8e710f5)2

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.

  2. No successful run was found on mk/lambda-variables (38efd3e) during the generation of this report, so db36371 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant