Skip to content

Trim child data on Arrow slice imports - #10379

Merged
palaska merged 1 commit into
developfrom
bp/arrow-import-trim-slices
Oct 7, 2026
Merged

palaska merged 1 commit into
developfrom
bp/arrow-import-trim-slices

Conversation

@palaska

@palaska palaska commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Sliced Arrow string, binary, and list arrays retain child data outside their selected rows. Imports now expose only the referenced data and rebase offsets to zero.

Changes

  • One offset helper serves canonical conversions and ArrowSession list imports.
  • Byte buffers remain shared, and recursive imports preserve extension fields.
  • Regression tests cover sliced arrays, both list offset widths, and empty list slices.

Measured example

A temporary Rust example reproduced the import code from parent commit 9324b04d4 and compared it with ArrowSession imports at ebf4bbfcb.
Strings contained value-{row}. Lists contained [row, row + 1] with non-nullable Int64 elements. Each slice selected two rows at the middle of its parent.

Arrow type Parent rows Before: nbytes() After: nbytes() Reduction
Utf8 1,000 8,902 B 30 B 99.66%
List<Int64> 1,000 16,012 B 44 B 99.73%
LargeList<Int64> 1,000 16,024 B 56 B 99.65%
List<Int64> 1,000,000 16,000,012 B 44 B 99.999725%
LargeList<Int64> 1,000,000 16,000,024 B 56 B 99.999650%

For the 1,000-row List<Int64> case, the old import exposed all 2,000 child values. The new import exposes only four child values.
The old size was 1,000 × 2 × 8 + 3 × 4 = 16,012 bytes. The new size is 2 × 2 × 8 + 3 × 4 = 44 bytes.
Both versions returned the same values as fresh two-row arrays. After the change, nbytes() also matched the fresh arrays in every case.

nbytes() counts logical buffer lengths, including offsets and children. These results do not measure RSS, file size, or runtime.
Arrow allocations remain shared, so the original allocation can remain resident.

Validation

  • The temporary example passed value-equality assertions for all five comparisons.
  • cargo test --locked -p vortex-arrow --lib test_sliced_: 12 passed, 0 failed.
  • Linters were not run.

Signed-off-by: Baris Palaska <barispalaska@gmail.com>
@palaska palaska added the changelog/fix A bug fix label Oct 7, 2026
@codspeed

codspeed Bot commented Oct 7, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 13.45%

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

⚠️ 1 benchmark measured no execution time

Nothing ran under measurement, usually because the compiler removed the code under test. This result is not comparable, so it counts as unchanged.

Preventing compiler optimizations

⚡ 1 improved benchmark
✅ 2122 untouched benchmarks
⏩ 518 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ WallTime bitpack_blocked_compress_avx2 7.6 µs 6.7 µs +13.45%
⚠️ Simulation bench_compare_sliced_dict_primitive[(3333, 10000)] 76 µs < 1 ns N/A

Tip

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


Comparing bp/arrow-import-trim-slices (ebf4bbf) with develop (09df3e8)

Open in CodSpeed

Footnotes

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

@joseph-isaacs

Copy link
Copy Markdown
Contributor

Should we do this for more types?

@palaska
palaska merged commit 80fe5d1 into develop Oct 7, 2026
150 of 152 checks passed
@palaska
palaska deleted the bp/arrow-import-trim-slices branch October 7, 2026 15:49
joseph-isaacs pushed a commit that referenced this pull request Oct 8, 2026
Arrow's Parquet reader points each batch's Utf8View/BinaryView views into
the whole decompressed page. from_arrow_byte_view imported those buffers
whole, so every batch kept, and nbytes() reported, every page it
touched. TPC-H l_comment batches of ~27 KiB of strings reported about
1 MiB each, which stopped the writer from coalescing them.

The import now walks the valid, non-inlined views once and per buffer:
- drops buffers no valid row references,
- slices buffers whose used range is at least half used, without copying,
- copies the values of sparsely used buffers.
Views of null rows become empty. Fully used buffers are still shared
zero-copy. This extends #10379, which trimmed sliced Utf8/Binary/List
imports, to view types.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QsHPGatG1T9kXdC662vKcm
Signed-off-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants