Remove variants module#8643
Conversation
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | chunked_varbinview_opt_canonical_into[(1000, 10)] |
169.6 µs | 206.4 µs | -17.85% |
| ❌ | Simulation | chunked_varbinview_into_canonical[(1000, 10)] |
169.4 µs | 205.8 µs | -17.68% |
| ❌ | Simulation | chunked_varbinview_opt_into_canonical[(1000, 10)] |
183.5 µs | 219.8 µs | -16.52% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
273.6 ns | 215.3 ns | +27.1% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[1024] |
333.9 ns | 275.6 ns | +21.17% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[2048] |
427.8 ns | 369.4 ns | +15.79% |
| ⚡ | Simulation | rebuild_naive |
105.5 µs | 91.3 µs | +15.47% |
| ⚡ | Simulation | chunked_varbinview_into_canonical[(100, 100)] |
306.6 µs | 272.1 µs | +12.68% |
| ⚡ | Simulation | encode_varbin[(1000, 32)] |
162.1 µs | 146.3 µs | +10.78% |
| 🆕 | Simulation | nonnull |
N/A | 302 µs | N/A |
| 🆕 | Simulation | nullable |
N/A | 315 µs | N/A |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rk/variants (ccfbcdb) with develop (5172987)
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. ↩
|
@codex please review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: Robert Kruszewski <github@robertk.io>
- Fix RefCell double-borrow panic in IndexOrd<Option<T>>::index_cmp: the RefMut guard from is_valid was still alive when value() re-borrowed the ctx, so searching any valid element with an optional needle panicked with BorrowMutError. Add regression tests covering Option<T> searches with and without nulls. - Stop swallowing the underlying error in Patches search_index_binary_search_scalar; the map_err replaced real execution errors with a misleading "indices must be a primitive array" message. - Pass the in-scope ExecutionCtx to find_physical_index in RunEnd scalar_at and slice instead of spinning up a fresh LEGACY_SESSION ctx per call. - Add doc comments to the new public SearchSortedPrimitiveArray API and document the null-as-zero semantics of plain T searches. - Restore VortexResult<()> + ? style in test_search_sorted_primitive. Signed-off-by: "Robert Kruszewski" <robert@spiraldb.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
I, Robert Kruszewski <github@robertk.io>, hereby add my Signed-off-by to this commit: 12082ae Signed-off-by: Robert Kruszewski <github@robertk.io>
I, Robert Kruszewski <github@robertk.io>, hereby add my Signed-off-by to this commit: 1d76252 Signed-off-by: Robert Kruszewski <github@robertk.io>
2 participants
This module was never fully fleshed out and we have different ways to accomplish same things. This also migrates the PValue search sorted to be purely primitive based