Skip to content

feat(array): scalar probes with one-off and repeated probe types - #9890

Closed
joseph-isaacs wants to merge 2 commits into
developfrom
ji/array-probe-enum-state
Closed

joseph-isaacs wants to merge 2 commits into
developfrom
ji/array-probe-enum-state

Conversation

@joseph-isaacs

@joseph-isaacs joseph-isaacs commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Scalar probes: a one-off row accessor that owns nothing, and a retained one that keeps encoding state, validity and child probes across rows. Two commits on develop:

  1. feat(array): add probe_scalar to OperationsVTable with a ProbeState shell, the mechanical part: type ProbeState and probe_scalar(state, index, ctx) on the vtable, defaulting to scalar_at; a ProbeState that only carries the ArrayView; the erased dispatch routed through it; type ProbeState = () in every encoding (44 files, one line each).
  2. feat(array): scalar probes with one-off and repeated probe types, the work to review: Probe, ArrayProbe, RepeatedArrayProbe, ProbeState with retained state and slot(), ProbeValidity, ArrayRef::probe / repeated_probe, Validity::probe, and the Struct / Primitive migration.

Why this shape

CodSpeed flagged is_valid_per_element and the struct execute_scalar benches at about −20% on the first design (#9843). The cost was destructors: every one-off read owned a temporary whose drop glue is recursive through the probe tree; LLVM emits that glue out of line, inlines nothing into unwind cleanup pads, and cannot fold a call it did not inline, so a provably empty value still pinned the temporary and blocked the tail call into the encoding. The rule that falls out: on the per-row path, hold only references across the call into the encoding. ArrayProbe is a &ArrayRef, ProbeState is a view plus Option<&mut _>, and slot() hands out an enum of references. The owning type, RepeatedArrayProbe, is only reached through &mut.

probe_scalar is deliberately not generic over the policy: a generic vtable method is instantiated in every crate that constructs arrays, where the encoding body loses this crate's inlining. Measured at 20 to 30 points on struct reads and rejected. scalar_at remains until encodings migrate. No unsafe, no ManuallyDrop.

Local numbers

Divan medians vs the develop tip, interleaved runs of 1000 samples, --profile bench:

bench develop this branch
execute_scalar_struct_simple 5.30 µs 3.78 µs (−29%)
execute_scalar_struct_wide 25.97 µs 15.94 µs (−39%)
is_valid_per_element[256] 3.89 µs 3.42 µs (−12%)
is_valid_per_element[1024] 15.23 µs 13.69 µs (−10%)

Checks

  • cargo +nightly fmt --all -- --check: clean
  • cargo clippy -p vortex-array --all-targets --all-features: clean
  • cargo nextest run -p vortex-array --cargo-profile bench --no-fail-fast: 3533 passed, 2 failed (listview should_panic tests gated on cfg!(debug_assertions), which the bench profile disables; pre-existing)
  • Doctests for vortex-array: pass

🤖 Generated with Claude Code

@codspeed

codspeed Bot commented Sep 15, 2026

Copy link
Copy Markdown

Merging this PR will regress 3 benchmarks

⚠️ 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.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
❌ 3 regressed benchmarks
✅ 2192 untouched benchmarks
⏩ 218 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation random_i8[0.5] 71.3 µs 94 µs -24.1%
Simulation new_raw_prim_test_between[i32, 2048] 64.1 µs 77.1 µs -16.84%
WallTime dict_canonicalize_gt_u8_neon[1000000] 487.8 µs 569.4 µs -14.32%
Simulation execute_scalar_struct_wide 503 µs 394.1 µs +27.62%
Simulation random_i16[0.8] 96.4 µs 77.5 µs +24.5%
Simulation execute_scalar_struct_simple 106.5 µs 90.3 µs +17.92%

Tip

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


Comparing ji/array-probe-enum-state (8a4beb1) with develop (b5f43ba)

Open in CodSpeed

Footnotes

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

@joseph-isaacs joseph-isaacs changed the title refactor(array): enum ProbeState with policy-free child reads (probe API, enum variant) feat(array): scalar probes with one-off and repeated probe types Sep 15, 2026
@joseph-isaacs joseph-isaacs added the changelog/break A breaking API change label Sep 15, 2026
…hell

Adds the vtable hook the scalar probe API builds on, without any of the
probe machinery. `OperationsVTable` gains `type ProbeState`, the state an
encoding may keep across repeated reads, and `probe_scalar(state, index,
ctx)`, defaulting to `scalar_at` so no encoding changes behaviour.
`ProbeState<'_, V>` for now only carries the `ArrayView`; the erased
dispatch routes one-off reads through it. Every encoding declares
`type ProbeState = ()`.

`scalar_at` stays until encodings migrate to `probe_scalar`.

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016CqrLKgPqFYGZK5sjk1qe7
Row access over arrays on top of the `probe_scalar` hook:

- `ArrayProbe<'a>` is a bare `&ArrayRef` with no destructor. One-off
  reads, including every child read while recursing through nested
  arrays, own nothing, so there is no unwind cleanup, no out-of-line drop
  glue and no pinned temporary on the per-row path. This is what the
  CodSpeed regression on the earlier design came down to.
- `RepeatedArrayProbe` owns its handle, encoding state, validity probe
  and child probes, created on first use and reused across rows.
- Both implement `Probe`. `ArrayRef::probe()` and
  `ArrayRef::repeated_probe()` are the entry points; `execute_scalar` and
  `is_valid` are shims over `probe()`.
- `ProbeState<'_, V>` carries the `ArrayView` and, for a repeated read, a
  borrow of the retained `RepeatedState`; it has no destructor.
  `state.array()`, `state.retained()`, `state.slot(i) -> impl Probe`, and
  the direct `child_scalar` / `child_is_valid`. Encodings never branch on
  policy for child reads.
- `Validity::probe()` gives the retained validity accessor.
- `Struct` and `Primitive` implement `probe_scalar`; `scalar_at`
  delegates through `ProbeState::once`.

Not generic over the retention policy: a generic vtable method is
instantiated in every crate that constructs arrays of the encoding, so
the hot body compiles downstream without this crate's inlining; measured
at 20-30 points on struct reads and rejected. The policy is a runtime
`Option` check, folded into the one-off arm by `inline(always)` on the
probe bodies.

Local divan medians vs the develop tip, interleaved runs of 1000
samples: is_valid_per_element[1024] -10%, [256] -12%,
execute_scalar_struct_wide -39%, execute_scalar_struct_simple -29%.

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016CqrLKgPqFYGZK5sjk1qe7
@joseph-isaacs
joseph-isaacs force-pushed the ji/array-probe-enum-state branch from c5f1c06 to 8a4beb1 Compare September 15, 2026 17:59
@joseph-isaacs

Copy link
Copy Markdown
Contributor Author

Superseded: this content is now the head of #9843.

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

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant