Thread ExecutionCtx into VTable::validate#8610
Conversation
Merging this PR will degrade performance by 40.76%
|
| 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)
Footnotes
-
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. ↩
5e975ca to
f8f6a90
Compare
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
7635ae6 to
c4687a7
Compare
Stacked on #8609.
Adds a
ctx: &mut ExecutionCtxparameter toVTable::validateand wires it throughArray::try_from_parts/ArrayInner::try_new, so encoding validation that needs canonicalization can use a real execution context instead of the hidden globalLEGACY_SESSION.Changes
VTable::validateand all impls (vortex-array + every encoding crate) gain the new finalctx/_ctxparameter. Most validations are pure metadata checks and ignore it (_ctx).RunEndandFSSTvalidatepreviously minted aLEGACY_SESSIONctx via aTODO(ctx); they now consume the threadedctx(removes 2LEGACY_SESSIONuses).try_from_partscallers thread an in-scopectx/sessionwhere one exists (RunEnd/ALPRDtry_new,deserialize/compresspaths use the availablesession).StructArray::try_new,DictArray::try_new,PrimitiveArray::from_buffer_handle, ALP/parquet-variant/fastlanes/zstdtry_new) mintLEGACY_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 forthcomingLEGACY_SESSIONallow-list.Verification
cargo build -p vortex-array --all-targets✅cargo test -p vortex-array✅ (3078 unit + 69 doctests)cargo testfor encodings (runend/fsst/alp/fastlanes) ✅cargo clippy --all-targets --all-features -- -D warningson vortex-array + all encodings ✅cargo +nightly fmt --all --check✅VTable::validatenor calltry_from_parts, so they are unaffected.(Workspace-wide
cargoruns 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