[danking]

Use the logical-row page index to skip unrelated pages when executing slices and compact range takes. Stream contiguous selections into the output and reuse decoded pages across ranges, avoiding full-array decompression while retaining the existing paths for files without an index.

[codspeed]

Merging this PR will improve performance by 58.26%

4 warnings, 1 note

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ Unable to measure the execution of 2 benchmarks

These benchmarks measured no execution time, so their results are not comparable and count as unchanged. This usually happens when the compiler removes the code under test.

Prevent compiler optimizations

See affected benchmarks

⚠️ 18 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

ℹ️ 40 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

⚡ 7 improved benchmarks
✅ 2458 untouched benchmarks
⏩ 12 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	CPU Simulation	selection[("ranges", true)]	2,074.4 µs	694.8 µs	×3
⚡	CPU Simulation	selection[("ranges", false)]	1,692.1 µs	697.8 µs	×2.4
⚡	CPU Simulation	selection[("near_full", true)]	540.1 ms	323.3 ms	+67.06%
⚡	CPU Simulation	chunked_dict_primitive_canonical_into[f64, (1000, 1000, 10)]	200.7 µs	146.4 µs	+37.1%
⚡	CPU Simulation	selection[("near_full", false)]	344.2 ms	287.7 ms	+19.61%
⚡	CPU Simulation	metadata_open_batch[false]	652.6 ms	577.7 ms	+12.97%
⚡	CPU Simulation	metadata_open_batch[true]	646.6 ms	582.5 ms	+11%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing dk/pco-selective-ranges (26de111) with dk/pco-selective-take (eea0ae2)}

Footnotes

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

[danking]

@claude review

[github-actions]

Claude finished @danking's task in 1m 25s —— View job

---

Review: Stream PCO slices and compact PiecewiseSequence range takes

• Read the PR diff against origin/dk/pco-selective-take
• Read take.rs, access.rs, the Pco::execute / decompress paths and the PiecewiseSequenceArray constructor
• Post review

I found no correctness bugs. Descriptor overflow, out-of-bounds and declared-length checks all happen in validate_runs before any output is allocated. PiecewiseSequenceArray::try_new only checks structure (children must be non-nullable unsigned integers), so the malformed-descriptor cases in rejects_invalid_indices_and_descriptors do reach the Pco guards. Zero-page sliced sources also work: the legacy branch of AccessState::span returns an all-false span with values 0..0, which stream_range handles. My notes are mostly about performance and coverage.

1. The new sliced execute path may explain the CodSpeed scan regressions (likely, not confirmed)

array.rs:285: every sliced Pco now goes through stream_range instead of PcoData::decompress. Per page, stream_range → AccessState::span → resolve_impl does two things:

• It runs unsliced_validity().slice(rows).execute_mask(..) for that page.
• It looks up array.page_index() on every loop iteration (take.rs:691).

The old path ran execute_mask once for the whole array and decoded touched pages sequentially. For scans that produce many small slices over nullable data with small pages, the per-page validity execution plus up-front zeroing could cost more than before. That matches the ~10–14% regressions in test_scan_filter_to_arrow_array[int|int_float] and test_repeated_scan_filter[int|int_float]. Base and head ran on different runner types, so this needs a same-runner A/B before anyone acknowledges it on CodSpeed. If the A/B confirms it, two possible fixes:

• Gate the new path on slice width, e.g. use stream_range only when the slice is a small fraction of the page.
• Execute the visible validity mask once and hand it to span.

2. Multi-run compact stride-1 descriptors don't stream

take.rs:148 / gather: only a single stride == 1 run reaches stream_range. With two or more stride-1 runs (the common "take a few slices" shape), gather_typed uses AccessState<true>. Every decoded page stays alive for the whole operation, and AllTrue spans are copied out of the cached PrimitiveArray rather than decoded into the output. For descriptors that cover most of the array, peak memory is about the full decoded array plus the output. The PR body says "Full pages decode into final spans", which is only true for the single-run case. Possible fixes:

• Run each stride-1 run through the stream_range decode-into-output logic, reusing one scratch buffer and decoder.
• Or reword the PR body to match what the code does.

3. The compact fallback threshold measures the wrong cost

take.rs:138-145: random is compared with descriptor_bytes(random). On the compact path grouped is always None, so no per-request sort or resolved descriptors are ever allocated. The real cost of stride > 1 runs is:

• a resolve per element (a partition_point whenever the page changes);
• keeping every touched decoded page alive.

For example, one run with stride ≈ page_len touches and retains every page while random stays small. In that case canonicalizing would be strictly cheaper, but the check never falls back. Many length-1, stride-1 runs (effectively random indices written as a PiecewiseSequence) also bypass the check completely. Consider reusing the page-touch estimate from prefer_canonical_primitive, or at least counting runs.len() plus the distinct pages a strided run spans.

4. Minor

• take.rs:65-67: indices.as_opt::<PiecewiseSequence>() is called twice. Binding it once (let sequence = indices.as_opt::<…>(); let compact = sequence.is_some();) is cleaner.
• primitive_usizes goes through checked_index, so invalid lengths or multipliers report "Pco take index is negative or exceeds usize". The message is misleading for those children. They're unsigned by construction, so this only matters for values wider than usize.
• slice.rs: the new vortex_ensure! in SliceReduce is fine. stream_range repeats the same check, so you could drop one of them.
• array.rs:285: crate::take::slice puts a slice-decode entry point inside the take module. It might sit more naturally in slice.rs or a small stream.rs next to stream_range.

5. Test coverage

• No test runs a PiecewiseSequence take on an already-sliced Pco, so slice_start != 0 combined with compact runs is untested. mixed_fragments_narrow_slices_and_compact_repeats slices only before a primitive take, and runs the compact descriptor on the unsliced array. One rstest case such as encoded.slice(17..200)?.take(piecewise) would cover the array.slice_start() + start arithmetic in the stride 0, 1 and >1 branches.
• No test has a multi-run stride-1 descriptor that crosses pages with mixed validity (the Mask::Values branch of gather_typed). The first piecewise_ranges case gets close, but the lengths (15, 20, 3) all fall within a few small pages.
• streamed_slices_preserve_float_bits_and_validity compares element by element. CLAUDE.md asks for assert_arrays_eq!, but bit-exact NaN comparison is a fair reason to deviate here. A one-line comment saying so would help.

Checks

I didn't run any builds, clippy or tests: the review was read-only, and CLAUDE.md leaves verification to the user.

[danking]

Addressed short-output handling in c6436a4: remove the truncate/zero-padding completion and return an error if the range output or streamed slice did not complete. These are constant-time output/progress checks, not whole-index or validity-popcount scans. Oversized range descriptors now have a descriptor-specific error message.

The requested non-nullable, sliced, and multi-page mixed-validity compact cases are in the earlier test-only PR10825. The earlier duplicate PiecewiseSequence lookup/legacy selective path are no longer present. Multi-run decoded-page retention remains the documented design; the optional wider-stride fallback tuning was not changed without measurements. The slice helper remains with the shared range decoder to avoid an unrelated module move. Local independent source review passed; tests and benchmarks have not been rerun.