Replace ToCanonical trait usages with execute by robert3005 · Pull Request #8609 · vortex-data/vortex

robert3005 · 2026-06-27T00:25:58Z

This also removes ArrayBuilder::extend_from_array and moves that logic
in to ArrayRef::append_to_builder

Plumb ExecutionCtx through more methods instead of creating fresh one.

Fixes: #3235

Signed-off-by: Robert Kruszewski github@robertk.io

codspeed-hq · 2026-06-27T00:29:56Z

Merging this PR will degrade performance by 12.14%

⚠️

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.

⚡ 2 improved benchmarks
❌ 8 regressed benchmarks
✅ 1587 untouched benchmarks
⏩ 4 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`extend_from_array_zctl[(1000, 8)]`	239.4 µs	404.3 µs	-40.79%
❌	Simulation	`extend_from_array_non_zctl_overlapping[(1000, 8)]`	278.5 µs	435.9 µs	-36.12%
❌	Simulation	`extend_from_array_non_zctl_overlapping[(1000, 32)]`	745.4 µs	897.4 µs	-16.93%
❌	Simulation	`slice_empty_vortex`	339.4 ns	397.8 ns	-14.66%
❌	Simulation	`extend_from_array_zctl[(1000, 64)]`	1 ms	1.2 ms	-13.84%
❌	Simulation	`extend_from_array_zctl[(10000, 8)]`	1.9 ms	2.2 ms	-10.73%
❌	Simulation	`extend_from_array_non_zctl_overlapping[(10000, 8)]`	2.2 ms	2.5 ms	-10.58%
❌	Simulation	`i32_small_overlapping`	40.2 µs	44.8 µs	-10.42%
⚡	Simulation	`bench_many_codes_few_values[1024]`	539.4 µs	364 µs	+48.17%
⚡	Simulation	`bitwise_not_vortex_buffer_mut[128]`	273.6 ns	244.4 ns	+11.93%

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 (53ed496) with develop (ee2cd67)}

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. ↩

Rebased onto develop and made standalone (no longer stacked on #8609). Instead of deprecating the hidden global session, expose it as `vortex_array::legacy_session()` and add it to the central `clippy.toml` `disallowed-methods` list (denied via `clippy::all`). New call sites are now rejected by clippy; every current invocation is explicitly allowlisted with `#[allow(clippy::disallowed_methods)]` on its enclosing function. - `LEGACY_SESSION` static -> `legacy_session() -> &'static VortexSession` (the `LazyLock` is now a private static inside the accessor). - All call sites renamed and allowlisted across vortex-array, the encodings, vortex-ffi, vortex-cuda, vortex-cxx, vortex-layout, vortex-python and vortex-web. develop has more call sites than #8609 (incl. test/builder code and crates #8609 had refactored away), all covered. Verified with: - cargo clippy -p vortex-array -p vortex-alp -p vortex-runend -p vortex-fsst -p vortex-fastlanes -p vortex-zstd -p vortex-ffi -p vortex-layout -p vortex-web-wasm -p vortex-cxx -p vortex-python --all-targets --all-features (0 disallowed-method hits, 0 warnings) - cargo +nightly fmt --check (touched crates) vortex-cuda could not be compiled here (CUDA toolchain unavailable); its two call sites are allowlisted following the same pattern. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018E5DywGCpq26ndoMHd1vyg

Rebased onto develop and made standalone (no longer stacked on #8609). Instead of deprecating the hidden global session, expose it as `vortex_array::legacy_session()` and add it to the central `clippy.toml` `disallowed-methods` list (denied via `clippy::all`). New call sites are now rejected by clippy; every current invocation is explicitly allowlisted with `#[allow(clippy::disallowed_methods)]` on its enclosing function. - `LEGACY_SESSION` static -> `legacy_session() -> &'static VortexSession` (the `LazyLock` is now a private static inside the accessor). - All call sites renamed and allowlisted across vortex-array, the encodings, vortex-ffi, vortex-cuda, vortex-cxx, vortex-layout, vortex-python and vortex-web. develop has more call sites than #8609 (incl. test/builder code and crates #8609 had refactored away), all covered. Verified with: - cargo clippy -p vortex-array -p vortex-alp -p vortex-runend -p vortex-fsst -p vortex-fastlanes -p vortex-zstd -p vortex-ffi -p vortex-layout -p vortex-web-wasm -p vortex-cxx -p vortex-python --all-targets --all-features (0 disallowed-method hits, 0 warnings) - cargo +nightly fmt --check (touched crates) vortex-cuda could not be compiled here (CUDA toolchain unavailable); its two call sites are allowlisted following the same pattern. Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018E5DywGCpq26ndoMHd1vyg Signed-off-by: Robert Kruszewski <github@robertk.io>

lwwmanning · 2026-07-02T16:54:13Z

@codex please review this change

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de998866cb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

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".

This also removes ArrayBuilder::extend_from_array and moves that logic in to ArrayRef::append_to_builder Plumb ExecutionCtx through more methods instead of creating fresh one. Signed-off-by: Robert Kruszewski <github@robertk.io>

Signed-off-by: Robert Kruszewski <github@robertk.io>

Replace the ListBuilder/ListViewBuilder downcast-enumeration macros with two dyn-dispatched ArrayBuilder methods, append_list_array and append_listview_array, taking ArrayViews. The builders resolve their own offset/size generics, so every IntegerPType (signed included) works, and either list encoding can append into either list builder. List-encoded sources now append directly from the ListArray layout without canonicalizing to ListViewArray first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io>

robert3005 requested review from a team, 0ax1, AdamGS and gatesn June 27, 2026 00:25

robert3005 added the changelog/chore A trivial change label Jun 27, 2026

This was referenced Jun 27, 2026

Thread ExecutionCtx into VTable::validate #8610

Closed

ALPRD: assert patch dtype instead of casting #8611

Closed

robert3005 force-pushed the claude/nice-hypatia-g68zgt branch from 5e975ca to f8f6a90 Compare June 27, 2026 15:12

lwwmanning reviewed Jun 27, 2026

View reviewed changes

Comment thread vortex-array/src/arrays/list/vtable/mod.rs Outdated

Comment thread encodings/runend/src/arbitrary.rs

robert3005 force-pushed the claude/nice-hypatia-g68zgt branch 2 times, most recently from 048b19b to 947a966 Compare June 27, 2026 20:53

robert3005 force-pushed the claude/nice-hypatia-g68zgt branch 2 times, most recently from bc088d1 to d2f276a Compare July 1, 2026 14:53

robert3005 enabled auto-merge (squash) July 1, 2026 14:53

robert3005 added changelog/break A breaking API change and removed changelog/chore A trivial change labels Jul 2, 2026

chatgpt-codex-connector Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread vortex-array/src/builders/mod.rs Outdated

Comment thread vortex-array/src/arrays/listview/vtable/mod.rs Outdated

robert3005 force-pushed the claude/nice-hypatia-g68zgt branch from 08472bb to 0871721 Compare July 2, 2026 21:16

robert3005 mentioned this pull request Jul 2, 2026

Make ArrayBuilder::extend_from_array return VortexResult #8311

Closed

claude and others added 3 commits July 2, 2026 23:50

Replace ToCanonical trait usages with execute

14d8fb9

This also removes ArrayBuilder::extend_from_array and moves that logic in to ArrayRef::append_to_builder Plumb ExecutionCtx through more methods instead of creating fresh one. Signed-off-by: Robert Kruszewski <github@robertk.io>

more

42efb87

Signed-off-by: Robert Kruszewski <github@robertk.io>

robert3005 force-pushed the claude/nice-hypatia-g68zgt branch from 0871721 to 53ed496 Compare July 2, 2026 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace ToCanonical trait usages with execute#8609

Replace ToCanonical trait usages with execute#8609
robert3005 wants to merge 3 commits into
developfrom
claude/nice-hypatia-g68zgt

robert3005 commented Jun 27, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

lwwmanning commented Jul 2, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

robert3005 commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will degrade performance by 12.14%

Performance Changes

Footnotes

Uh oh!

Uh oh!

Uh oh!

lwwmanning commented Jul 2, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robert3005 commented Jun 27, 2026 •

edited

Loading

codspeed-hq Bot commented Jun 27, 2026 •

edited

Loading