Skip to content

Thread ExecutionCtx into VTable::validate#8610

Closed
robert3005 wants to merge 1 commit into
claude/nice-hypatia-g68zgtfrom
claude/nice-hypatia-g68zgt-validate-ctx
Closed

Thread ExecutionCtx into VTable::validate#8610
robert3005 wants to merge 1 commit into
claude/nice-hypatia-g68zgtfrom
claude/nice-hypatia-g68zgt-validate-ctx

Conversation

@robert3005

Copy link
Copy Markdown
Contributor

Stacked on #8609.

Adds a ctx: &mut ExecutionCtx parameter to VTable::validate and wires it through Array::try_from_parts / ArrayInner::try_new, so encoding validation that needs canonicalization can use a real execution context instead of the hidden global LEGACY_SESSION.

Changes

  • VTable::validate and all impls (vortex-array + every encoding crate) gain the new final ctx/_ctx parameter. Most validations are pure metadata checks and ignore it (_ctx).
  • RunEnd and FSST validate previously minted a LEGACY_SESSION ctx via a TODO(ctx); they now consume the threaded ctx (removes 2 LEGACY_SESSION uses).
  • try_from_parts callers thread an in-scope ctx/session where one exists (RunEnd/ALPRD try_new, deserialize/compress paths use the available session).
  • Constructors with no context yet (e.g. StructArray::try_new, DictArray::try_new, PrimitiveArray::from_buffer_handle, ALP/parquet-variant/fastlanes/zstd try_new) mint LEGACY_SESSION.create_execution_ctx() as intermediate scaffolding (+17 sites). These are candidates for follow-up ctx threading (e.g. the ALP/ALPRD PR) and will be covered by the forthcoming LEGACY_SESSION allow-list.

Verification

  • cargo build -p vortex-array --all-targets
  • cargo test -p vortex-array ✅ (3078 unit + 69 doctests)
  • cargo test for encodings (runend/fsst/alp/fastlanes) ✅
  • cargo clippy --all-targets --all-features -- -D warnings on vortex-array + all encodings ✅
  • cargo +nightly fmt --all --check
  • Downstream crates (vortex-layout/file/scan/datafusion/…) build; they neither implement VTable::validate nor call try_from_parts, so they are unaffected.

(Workspace-wide cargo runs locally exclude the CUDA/duckdb/tpchgen crates, which need toolchains/network not available in this environment.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01Mkrgj5SuJpaKBXK9jydtSc


Generated by Claude Code

@codspeed-hq

codspeed-hq Bot commented Jun 27, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 40.76%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 1 improved benchmark
❌ 8 regressed benchmarks
✅ 1586 untouched benchmarks
⏩ 4 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation take_indices[(1000, 4)] 49.4 µs 185.2 µs -73.35%
Simulation take[sorted_linear_len16384_run8_take512] 66 µs 205.2 µs -67.84%
Simulation take_fsl_nullable_random[16, 100] 72.3 µs 209.1 µs -65.43%
Simulation chunked_bool_canonical_into[(1000, 10)] 16.2 µs 26.4 µs -38.4%
Simulation take_filter_primitive_large_random_mask_random_indices[(2500, 25000)] 283.9 µs 422.2 µs -32.75%
Simulation chunked_varbinview_opt_canonical_into[(1000, 10)] 169.6 µs 206 µs -17.67%
Simulation chunked_varbinview_opt_into_canonical[(1000, 10)] 183 µs 219.6 µs -16.66%
Simulation compress[(10000, 4)] 199 µs 227 µs -12.36%
Simulation chunked_varbinview_into_canonical[(1000, 10)] 205.7 µs 168.9 µs +21.8%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/nice-hypatia-g68zgt-validate-ctx (c4687a7) with claude/nice-hypatia-g68zgt (f8f6a90)

Open in CodSpeed

Footnotes

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

Add a `ctx: &mut ExecutionCtx` parameter to `VTable::validate` and wire it
through `Array::try_from_parts` / `ArrayInner::try_new`, so encoding validation
that needs canonicalization can use a real execution context.

`ALPRD::try_new` no longer takes a `ctx`: it mints a `LEGACY_SESSION` context
internally for its patch canonicalization and the `try_from_parts` validation,
matching the other constructors that have no ctx in scope (and grandfathered by
the LEGACY_SESSION allow-list). Its callers (filter/mask/take/encode) and the
btrblocks ALPRD scheme drop the trailing `ctx` argument accordingly.

Signed-off-by: Robert <robert@spiraldb.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mkrgj5SuJpaKBXK9jydtSc
@robert3005 robert3005 force-pushed the claude/nice-hypatia-g68zgt-validate-ctx branch from 7635ae6 to c4687a7 Compare June 27, 2026 15:57
@robert3005 robert3005 closed this Jun 27, 2026
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.

2 participants