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 2 benchmarks

⚡ 18 improved benchmarks
❌ 2 regressed benchmarks
✅ 1650 untouched benchmarks
🆕 128 new benchmarks
⏩ 44 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`copy_nullable[65536]`	1 ms	1.4 ms	-24.31%
❌	Simulation	`copy_non_nullable[65536]`	908.5 µs	1,089.7 µs	-16.63%
⚡	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,568.2 µs	794.8 µs	+97.3%
⚡	Simulation	`take_fsl_nullable_random[1024, 100]`	1,535.6 µs	804.9 µs	+90.78%
⚡	Simulation	`take_fsl_random[64, 1000]`	815.9 µs	456.9 µs	+78.58%
⚡	Simulation	`take_fsl_random[256, 100]`	418.5 µs	234.8 µs	+78.24%
⚡	Simulation	`take_fsl_nullable_random[64, 1000]`	812 µs	472.4 µs	+71.87%
⚡	Simulation	`take_fsl_nullable_random[256, 100]`	415.6 µs	244.9 µs	+69.71%
⚡	Simulation	`take_fsl_random[16, 1000]`	238.1 µs	179 µs	+32.99%
⚡	Simulation	`take_fsl_nullable_random[16, 1000]`	257.7 µs	194.3 µs	+32.63%
⚡	Simulation	`take_fsl_random[64, 100]`	123.3 µs	94.6 µs	+30.28%
⚡	Simulation	`take_fsl_random[4096, 1000]`	67.3 ms	54.2 ms	+24.2%
⚡	Simulation	`take_fsl_nullable_random[64, 100]`	131.6 µs	107.4 µs	+22.53%
⚡	Simulation	`take_fsl_nullable_random[4096, 1000]`	65.6 ms	54.2 ms	+20.92%
⚡	Simulation	`true_count_vortex_buffer[128]`	580.6 ns	522.2 ns	+11.17%
⚡	Simulation	`take_fsl_random[4096, 100]`	6.2 ms	5.6 ms	+10.07%
...	...	...	...	...	...

ℹ️ 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 (10ef731) with develop (05c7c2f)}

44 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

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.

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

I cannot replicate the two benchmark slowdowns locally.

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

The copy_ benchmark slowdowns are resolved by #8832

robert3005 · 2026-07-18T00:23:50Z

+    let validity = array.validity()?.take(indices_ref)?;
+    let output_len = indices_ref.len();
+    let taken = match lengths {
+        ConstantOrArray::Constant(length) => {


we have a thing that we call Columnar

OK, switched to that.

robert3005 · 2026-07-18T00:24:11Z

+    indices_ref: &ArrayRef,
+    ctx: &mut ExecutionCtx,
+) -> VortexResult<Option<ArrayRef>> {
+    let Some((starts, lengths)) = maybe_contiguous_slices(indices, ctx)? else {


Maybe minor but in this method you double validate if this succeeds

I think this is resolved by de798ad

robert3005 · 2026-07-18T00:25:27Z

+    check_index_arrays(array.starts(), array.lengths(), array.multipliers())?;
+    let starts = array.starts().clone().execute::<PrimitiveArray>(ctx)?;
+    if let Some(length) = constant_unsigned_usize(array.lengths())? {
+        check_index_arrays(starts.as_ref(), array.lengths(), array.multipliers())?;
+        return Ok(Some((starts, ConstantOrArray::Constant(length))));
+    }
+
+    let lengths = array.lengths().clone().execute::<PrimitiveArray>(ctx)?;
+    check_index_arrays(starts.as_ref(), lengths.as_ref(), array.multipliers())?;


I think I miss something - why we validate so hard here?

Just Codex being an idiot. Cleaning up now.

Done: de798ad

robert3005 · 2026-07-18T00:25:48Z

+    check_index_arrays(array.starts(), array.lengths(), array.multipliers())?;
+    let starts = array.starts().clone().execute::<PrimitiveArray>(ctx)?;
+    if let Some(length) = constant_unsigned_usize(array.lengths())? {
+        check_index_arrays(starts.as_ref(), array.lengths(), array.multipliers())?;
+        return Ok(Some((starts, ConstantOrArray::Constant(length))));
+    }
+
+    let lengths = array.lengths().clone().execute::<PrimitiveArray>(ctx)?;
+    check_index_arrays(starts.as_ref(), lengths.as_ref(), array.multipliers())?;


this should be enough

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

github-actions · 2026-07-20T16:26:18Z

Polar Signals Profiling Results

Latest Run

Status	Commit	Job	Attempt	Link
🟢 Done	`10ef731`		1	Explore Profiling Data

Previous Runs (3)

Status	Commit	Attempt	Link
🟢 Done	`de798ad`	1	Explore Profiling Data
🟢 Done	`210edda`	1	Explore Profiling Data
🟢 Done	`1fdda1c`	1	Explore Profiling Data

Powered by Polar Signals Cloud

github-actions · 2026-07-20T16:27:12Z

Benchmarks: Vortex queries 📖

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +1.9%
Engines: DataFusion No clear signal (+7.5%, low confidence) · DuckDB No clear signal (-3.5%, low confidence)
Vortex (geomean): 0.978x ➖
Parquet (geomean): 0.969x ➖
Shifts: Parquet (control) -3.1% · Median polish -2.6%

How to read Verdict and Engines

Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.

datafusion / vortex-file-compressed (1.018x ➖, 0↑ 0↓)

name	PR `10ef731` (ns)	base `05c7c2f` (ns)	ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed	9539375	9447028	1.01
vortex_q01/datafusion:vortex-file-compressed	6294393	6130391	1.03

datafusion / parquet (0.947x ➖, 0↑ 0↓)

name	PR `10ef731` (ns)	base `05c7c2f` (ns)	ratio (PR/base)
vortex_q00/datafusion:parquet	19756785	20931306	0.94
vortex_q01/datafusion:parquet	4574112	4811001	0.95

duckdb / vortex-file-compressed (0.957x ➖, 0↑ 0↓)

name	PR `10ef731` (ns)	base `05c7c2f` (ns)	ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed	9863733	10304025	0.96
vortex_q01/duckdb:vortex-file-compressed	5955384	6223625	0.96

duckdb / parquet (0.992x ➖, 0↑ 0↓)

name	PR `10ef731` (ns)	base `05c7c2f` (ns)	ratio (PR/base)
vortex_q00/duckdb:parquet	23266656	23586269	0.99
vortex_q01/duckdb:parquet	9430211	9459430	1.00

No file size changes detected.

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

## Summary - move VarBin and VarBinView PiecewiseSequence take specializations out of #8800 into this stacked PR - keep the branch based directly on codex/piecewise-sequential-take - preserve existing behavior: VarBin copies contiguous byte ranges, VarBinView gathers view ranges while reusing backing data buffers ## Validation - cargo +nightly fmt --all - cargo check -p vortex-array --------- 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