Use PiecewiseSequence indices for FSL take by danking · Pull Request #8800 · vortex-data/vortex

danking · 2026-07-16T17:43:58Z

Summary

Ports the TakeSlices-style optimization to Take(elements, PiecewiseSequenceArray) instead of introducing a value-wrapper API.

FSL take now builds one start per selected list, a constant list length, and a constant multiplier of 1, then calls elements.take(piecewise_indices).
Primitive, bool, decimal, VarBinView, and VarBin take paths detect PiecewiseSequenceArray indices and gather contiguous ranges only when multipliers is a constant unsigned 1; non-unit multipliers fall back through normal materialized-index take.
Shared PiecewiseSequence execution handles general start + j * multiplier sequences, while the current optimized consumers stay explicitly contiguous.

Validation

cargo fmt -p vortex-array
cargo test -p vortex-array piecewise_sequence
cargo check -p vortex-array
cargo check -p vortex-array --benches
git diff --check

codspeed-hq · 2026-07-16T18:02:01Z

Merging this PR will regress 1 benchmark

⚡ 19 improved benchmarks
❌ 1 regressed benchmark
✅ 1650 untouched benchmarks
🆕 128 new benchmarks
⏩ 52 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`true_count_vortex_buffer[128]`	522.2 ns	580.6 ns	-10.05%
⚡	Simulation	`take_fsl_random[1024, 1000]`	15.2 ms	6 ms	×2.5
⚡	Simulation	`take_fsl_nullable_random[1024, 1000]`	14.8 ms	6 ms	×2.5
⚡	Simulation	`take_fsl_random[256, 1000]`	3.5 ms	1.5 ms	×2.2
⚡	Simulation	`take_fsl_nullable_random[256, 1000]`	3.4 ms	1.5 ms	×2.2
⚡	Simulation	`take_fsl_random[1024, 100]`	1,567.8 µs	795.2 µs	+97.14%
⚡	Simulation	`take_fsl_nullable_random[1024, 100]`	1,535.6 µs	805.7 µs	+90.6%
⚡	Simulation	`take_fsl_random[64, 1000]`	815.6 µs	457.3 µs	+78.35%
⚡	Simulation	`take_fsl_random[256, 100]`	418.1 µs	235.2 µs	+77.73%
⚡	Simulation	`take_fsl_nullable_random[64, 1000]`	811.9 µs	473.1 µs	+71.6%
⚡	Simulation	`take_fsl_nullable_random[256, 100]`	415.5 µs	245.6 µs	+69.17%
⚡	Simulation	`take_fsl_random[16, 1000]`	237.7 µs	179.4 µs	+32.5%
⚡	Simulation	`take_fsl_nullable_random[16, 1000]`	257.6 µs	195 µs	+32.11%
⚡	Simulation	`take_fsl_random[64, 100]`	123 µs	95 µs	+29.36%
⚡	Simulation	`take_fsl_random[4096, 1000]`	67.3 ms	54.2 ms	+24.2%
⚡	Simulation	`take_fsl_nullable_random[64, 100]`	131.5 µs	108.1 µs	+21.67%
⚡	Simulation	`take_fsl_nullable_random[4096, 1000]`	65.6 ms	54.2 ms	+20.93%
⚡	Simulation	`compact_sliced[(4096, 90)]`	867.8 ns	780.3 ns	+11.21%
⚡	Simulation	`compact_sliced[(16384, 90)]`	931.1 ns	843.6 ns	+10.37%
⚡	Simulation	`take_fsl_random[4096, 100]`	6.2 ms	5.6 ms	+10.17%
...	...	...	...	...	...

ℹ️ 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 codex/piecewise-sequential-take (c94c4ef) with codex/piecewise-sequential-array (2b6fa3e)}

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

robert3005 · 2026-07-16T22:59:06Z

+    S: crate::dtype::UnsignedPType,
+    L: crate::dtype::UnsignedPType,
+{
+    validate_index_ranges(source.len(), starts, lengths, output_len)?;


the slice will validate them

robert3005 · 2026-07-16T22:59:55Z

+                    array.buffer::<D>().as_slice(),
+                    indices_ref.len(),
+                )?;
+                let validity = array.validity()?.take(indices_ref)?;


eh, we need my refactor pr to extract the ref from view

robert3005 · 2026-07-16T23:01:31Z

+    L: crate::dtype::UnsignedPType,
+    T: NativeDecimalType,
+{
+    validate_index_ranges(values.len(), starts, lengths, output_len)?;


this is already validated by the result slice

so imho you either keep this and convert the loop below all into unsafe accesses or remove this and use the optional getters on slice

robert3005 · 2026-07-16T23:03:18Z

+    for (&start, &length) in starts.iter().zip_eq(lengths) {
+        let start = start.as_();
+        let length = length.as_();
+        result.extend_from_slice(&values[start..start + length]);


I think this would be better if you had the mutable result and then wrote directly into the next available range result[curr..][..length]

So, I was a bit skeptical this would help (surely a sufficiently advanced compiler...).

I ran a benchmark for any subset of:

replace extend_from_slice with mutable_result[cursor..][..length].copy_slice_from

Verify all starts & starts + length are valid indices in a previous loop and use unsafe unchecked slicing.

On these data:

Source data is just random u16s of size 1,000, 10,000, 100,000.

Number of slices is 50.

Slice lengths are normally distributed with mean 1/100 of source data length.

Starts are uniformly randomly chosen but then sorted.

The cursor approach does seem noticeably better, particularly when the slices are mean 200 bytes. Also better for slices of mean 20 bytes but less significantly so.

source len extend cursor extend unchecked cursor unchecked

1,000 275.7 ns, 3.60 GB/s 249.7 ns, 3.98 GB/s 283.6 ns, 3.50 GB/s 260.1 ns, 3.82 GB/s

10,000 247.1 ns, 38.3 GB/s 208.0 ns, 45.5 GB/s 254.9 ns, 37.1 GB/s 223.7 ns, 42.3 GB/s

100,000 2.020 µs, 47.7 GB/s 2.062 µs, 46.8 GB/s 2.020 µs, 47.7 GB/s 2.062 µs, 46.8 GB/s

I'm gonna create a follow up PR that address this.

robert3005 · 2026-07-16T23:08:12Z

+
+    let (new_elements, new_len) =
+        take_non_empty_non_degenerate_elements::<I>(array, indices_array, ctx)?;
+    let new_validity = if array.dtype().is_nullable() || indices_nullability.is_nullable() {


I think validity take already handles this

robert3005

I think we have to audit the validation from the compute and verify that we do not do the work twice. We have to audit exactly but ideally we'd not validate and figure out how to make operations safe by default even given bad input.

Signed-off-by: Daniel King <dan@spiraldb.com>

danking · 2026-07-17T19:38:55Z

I cannot replicate the two benchmark slowdowns locally.

Signed-off-by: Daniel King <dan@spiraldb.com>

danking · 2026-07-17T20:20:11Z

The copy_ benchmark slowdowns are resolved by #8832

Signed-off-by: Daniel King <dan@spiraldb.com>

danking force-pushed the codex/piecewise-sequential-array branch from aba7e4d to 3edf6c8 Compare July 16, 2026 18:06

danking force-pushed the codex/piecewise-sequential-take branch from 07fde63 to 3ecd955 Compare July 16, 2026 18:06

danking force-pushed the codex/piecewise-sequential-array branch from 3edf6c8 to 1748e31 Compare July 16, 2026 18:20

danking force-pushed the codex/piecewise-sequential-take branch from 3ecd955 to 98a9180 Compare July 16, 2026 18:20

danking mentioned this pull request Jul 16, 2026

Add TakeSlices for fixed-size-list take #8674

Closed

danking force-pushed the codex/piecewise-sequential-take branch from 01e95cb to eb83451 Compare July 16, 2026 19:19

danking changed the title ~~Use PiecewiseSequential indices for FSL take~~ Use PiecewiseSequence indices for FSL take Jul 16, 2026

danking force-pushed the codex/piecewise-sequential-take branch from eb83451 to e79ec4b Compare July 16, 2026 20:29

robert3005 reviewed Jul 16, 2026

View reviewed changes

danking force-pushed the codex/piecewise-sequential-array branch from 6eb0c92 to 16ca5dc Compare July 16, 2026 23:36

danking force-pushed the codex/piecewise-sequential-take branch from b615895 to 6dcc52a Compare July 16, 2026 23:41

danking added the changelog/performance A performance improvement label Jul 16, 2026

danking force-pushed the codex/piecewise-sequential-take branch from 6dcc52a to a2cbdf0 Compare July 17, 2026 00:15

danking added 7 commits July 16, 2026 20:17

Use PiecewiseSequential indices for FSL take

b9653f4

Signed-off-by: Daniel King <dan@spiraldb.com>

Port FSL take benchmarks to PiecewiseSequential

da98e61

Signed-off-by: Daniel King <dan@spiraldb.com>

Gate PiecewiseSequence take paths on unit multipliers

ca7d423

Signed-off-by: Daniel King <dan@spiraldb.com>

Preserve existing FSL take benchmarks

6ba9fe5

Signed-off-by: Daniel King <dan@spiraldb.com>

Split PiecewiseSequence take dispatch

9ceff27

Signed-off-by: Daniel King <dan@spiraldb.com>

Fix PiecewiseSequence test length cast

7cd3098

Signed-off-by: Daniel King <dan@spiraldb.com>

Use imported UnsignedPType in take consumers

d694338

Signed-off-by: Daniel King <dan@spiraldb.com>

danking force-pushed the codex/piecewise-sequential-take branch from a2cbdf0 to d694338 Compare July 17, 2026 00:17

danking mentioned this pull request Jul 17, 2026

Add PiecewiseSequence index array #8799

Open

danking added 2 commits July 17, 2026 11:28

Specialize constant PiecewiseSequence lengths

7c4811b

Signed-off-by: Daniel King <dan@spiraldb.com>

Rename contiguous range take helpers

ad5d6ff

Signed-off-by: Daniel King <dan@spiraldb.com>

Optimize FixedSizeList take results

2a33748

Signed-off-by: Daniel King <dan@spiraldb.com>

danking force-pushed the codex/piecewise-sequential-take branch from 89e4da2 to 2a33748 Compare July 17, 2026 15:43

danking added 2 commits July 17, 2026 14:39

Inline PiecewiseSequence range length checks

815e475

Signed-off-by: Daniel King <dan@spiraldb.com>

Simplify PiecewiseSequence take range handling

66a5896

Signed-off-by: Daniel King <dan@spiraldb.com>

danking added 3 commits July 17, 2026 15:38

Validate PiecewiseSequence buffer lengths after copy

18ec1b6

Signed-off-by: Daniel King <dan@spiraldb.com>

Cover null FSL take placeholder indices

699f48a

Signed-off-by: Daniel King <dan@spiraldb.com>

Rename contiguous PiecewiseSequence helpers

a2cf571

Signed-off-by: Daniel King <dan@spiraldb.com>

Move VarBin PiecewiseSequence takes out of base PR

5738804

Signed-off-by: Daniel King <dan@spiraldb.com>

danking mentioned this pull request Jul 17, 2026

Specialize VarBin PiecewiseSequence takes #8836

Draft

danking added 2 commits July 17, 2026 16:36

Fix VarBin take conformance tests

6ee68b3

Signed-off-by: Daniel King <dan@spiraldb.com>

Rename PiecewiseSequence constant helper

c94c4ef

Signed-off-by: Daniel King <dan@spiraldb.com>

source len	extend	cursor	extend unchecked	cursor unchecked
1,000	275.7 ns, 3.60 GB/s	249.7 ns, 3.98 GB/s	283.6 ns, 3.50 GB/s	260.1 ns, 3.82 GB/s
10,000	247.1 ns, 38.3 GB/s	208.0 ns, 45.5 GB/s	254.9 ns, 37.1 GB/s	223.7 ns, 42.3 GB/s
100,000	2.020 µs, 47.7 GB/s	2.062 µs, 46.8 GB/s	2.020 µs, 47.7 GB/s	2.062 µs, 46.8 GB/s

Uh oh!

Conversation

danking commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Uh oh!

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

Uh oh!

Merging this PR will regress 1 benchmark

Performance Changes

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robert3005 left a comment

Choose a reason for hiding this comment

Uh oh!

danking commented Jul 17, 2026

Uh oh!

danking commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danking commented Jul 16, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 16, 2026 •

edited

Loading