Skip to content

Support runend canonicalization for lists - #9708

Merged
danking merged 2 commits into
developfrom
dk/runend-list-canonicalize
Sep 3, 2026
Merged

Support runend canonicalization for lists#9708
danking merged 2 commits into
developfrom
dk/runend-list-canonicalize

Conversation

@danking

@danking danking commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add list dtype support in run_end_canonicalize via run index expansion and the existing take path
  • cover list(i64), list(bool), and list(utf8) run-end canonicalization

Tests

  • cargo test -p vortex-runend test_runend_list -- --nocapture
  • cargo test -p vortex-runend
  • cargo clippy -p vortex-runend --all-targets --all-features
  • cargo +nightly fmt --all

Comment thread encodings/runend/src/array.rs Outdated
@danking
danking force-pushed the dk/runend-list-canonicalize branch 2 times, most recently from 78849fb to cec8d20 Compare August 31, 2026 22:56
@codspeed-hq

codspeed-hq Bot commented Aug 31, 2026

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

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

⚡ 6 improved benchmarks
❌ 1 regressed benchmark
✅ 2174 untouched benchmarks
⏩ 206 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation random_i8[0.8] 68.9 µs 98.2 µs -29.86%
Simulation random_i16[0.8] 92.1 µs 74 µs +24.45%
WallTime words_gather_scalar_avx2[65536] 9.3 µs 8.2 µs +13.49%
Simulation allocate_drop_arrow[0] 456.9 ns 402.7 ns +13.45%
Simulation allocate_drop_bytes[0] 520.2 ns 466 ns +11.62%
WallTime mul_i32_nonnull_avx512 7.9 µs 7.1 µs +11.59%
WallTime mul_u32_nonnull_avx512 6.3 µs 5.7 µs +10.61%

Tip

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


Comparing dk/runend-list-canonicalize (d7ddd10) with develop (dab1684)

Open in CodSpeed

Footnotes

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

Signed-off-by: Daniel King <dan@spiraldb.com>
@danking
danking force-pushed the dk/runend-list-canonicalize branch from cec8d20 to 692c7e6 Compare August 31, 2026 23:16
@danking
danking marked this pull request as ready for review August 31, 2026 23:32
@danking
danking enabled auto-merge (squash) August 31, 2026 23:32
Comment thread encodings/runend/src/array.rs Outdated
Comment on lines +539 to +546
Validity::Array(validity) => Validity::Array(runend_decode_bools(
ends,
validity.execute_as::<BoolArray>("validity", ctx)?,
offset,
length,
ctx,
)?),
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess you could leave the validity as a run end array?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joseph-isaacs I actually wanted to ask you about that!

Would it be copacetic to simply recreate three RunEndArray's for the offsets, the sizes, and the validity? Or is that not what this method is supposed to do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to just push the RunEnd into the offsets, sizes, and validity.

Comment thread encodings/runend/src/array.rs Outdated
Comment on lines +528 to +533
let decoded_offsets =
runend_decode_primitive(ends.clone(), offsets, offset, length, ctx)?.into_array();

let sizes = values.sizes().clone().execute_as("sizes", ctx)?;
let decoded_sizes =
runend_decode_primitive(ends.clone(), sizes, offset, length, ctx)?.into_array();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if its worth fusing these two decode, seem like it will be much faster?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is now made irrelevant though this is still an interesting question.

Signed-off-by: Daniel King <dan@spiraldb.com>
@danking
danking merged commit 4161afd into develop Sep 3, 2026
94 of 95 checks passed
@danking
danking deleted the dk/runend-list-canonicalize branch September 3, 2026 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants