refactor(array): pass the execution context into patch lookups - #9996
joseph-isaacs wants to merge 1 commit into
Conversation
`Patches::get_patched` and `search_index` each built their own execution context from the legacy session, even though every caller of `get_patched` is a `scalar_at` that already holds one. Thread the caller's context through instead, down to the binary search over the indices, so a patch lookup reads with the context it was invoked under and builds none of its own. `Patches::slice` has no caller context, so it creates one legacy context and reuses it for both index searches and the chunk base read. 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_01ACe1mztjw9vNFUYwzVhGwz
Merging this PR will regress 11 benchmarks
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | filtered_sink_i64_avx512[OneNullInEight] |
22.3 µs | 31.7 µs | -29.64% |
| ❌ | words_gather_dispatch_avx512[65536] |
987 ns | 1,352 ns | -27% |
| ❌ | filtered_sink_i64_avx2[NineNullsInTen] |
14.8 µs | 18.3 µs | -19.29% |
| ❌ | compare_u64_neon |
4.7 µs | 5.8 µs | -18.88% |
| ❌ | filtered_sink_i64_avx512[NineNullsInTen] |
15 µs | 18.4 µs | -18.23% |
| ❌ | filtered_sink_i64_avx2[OneNullInEight] |
26.1 µs | 31.2 µs | -16.3% |
| ❌ | multiply_shapes_neon[(16384, PerRowPerRow)] |
17.4 µs | 20.4 µs | -14.57% |
| ❌ | compare_int_constant_neon |
4.1 µs | 4.7 µs | -12.04% |
| ❌ | words_gather_scalar_avx2[65536] |
8.2 µs | 9.4 µs | -11.98% |
| ❌ | compare_int_neon |
4.6 µs | 5.2 µs | -10.72% |
| ❌ | compare_int_eq_neon |
4.7 µs | 5.2 µs | -10.27% |
| ⚡ | mul_i16_nonnull_avx2 |
28.9 µs | 11.6 µs | ×2.5 |
| ⚡ | mul_i32_nonnull_avx2 |
32.9 µs | 13.4 µs | ×2.5 |
| ⚡ | mul_i8_nonnull_avx2 |
29.9 µs | 12.5 µs | ×2.4 |
| ⚡ | mul_u8_nonnull_avx512 |
22.5 µs | 9.6 µs | ×2.3 |
| ⚡ | mul_i32_nullable_avx2 |
34.4 µs | 14.9 µs | ×2.3 |
| ⚡ | mul_i8_nonnull_neon |
25.7 µs | 11.1 µs | ×2.3 |
| ⚡ | mul_i32_nonnull_neon |
23.8 µs | 10.4 µs | ×2.3 |
| ⚡ | mul_i16_nonnull_neon |
24.2 µs | 10.6 µs | ×2.3 |
| ⚡ | mul_u64_nonnull_avx2 |
39.3 µs | 17.4 µs | ×2.3 |
| ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/context-passing-parent-fo4x8u (c550195) with ji/patches-chunk-offset-probe (2e0df78)
Footnotes
-
2366 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. ↩
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Stacked on #9904. Follows up the review there by threading the caller's execution context through every patch lookup, rather than only into
search_index_chunked.Patches::get_patchedandsearch_indexeach built their own execution context from the legacy session, even though every caller ofget_patchedis ascalar_atthat already holds one.Changes
Patches::get_patched,search_index,search_index_chunkedand the private binary-search helpers takectx: &mut ExecutionCtxinstead of callinglegacy_session().create_execution_ctx(). Theclippy::disallowed_methodsallowances on those paths go away.scalar_atcallers (ALP, ALP-RD, BitPacked, Sparse) pass their context through; two of them previously bound it as_ctx.Patches::slicehas no caller context, so it creates one legacy context at the top and reuses it for both index searches and the chunk base read.patches_lookupbench and the patches tests create a context once and pass it in.API Changes
Patches::get_patchedandPatches::search_indexgain actx: &mut ExecutionCtxparameter.🤖 Generated with Claude Code
https://claude.ai/code/session_01ACe1mztjw9vNFUYwzVhGwz
Generated by Claude Code