Skip to content

perf(layout): stop retaining segments across V2 scans - #11408

Draft
joseph-isaacs wants to merge 18 commits into
developfrom
ji/no-retained-segments
Draft

joseph-isaacs wants to merge 18 commits into
developfrom
ji/no-retained-segments

Conversation

@joseph-isaacs

Copy link
Copy Markdown
Contributor

Summary

Experiment, stacked on #11377 (which includes #11399). It asks whether the V2 scan's RetainedSegments cache can be removed outright instead of made faster. Only the last commit, b312ac3, is new here. The benchmark bot compares against develop, so judge this PR by its results next to #11377's head (83cb452, whose code is the same as fc29d86).

RetainedSegments keeps up to 64 MiB of recently read segment bytes per file. It exists so chunks of a scan that share a stored segment read it once. On develop, chunks launch spaced apart, so a later chunk often starts after its neighbour's read finished and would read the segment again. #11377 launches a take's chunks together. Their requests for a shared segment then overlap in flight, and the file's segment source merges them, so the retention may no longer earn its memory and its lookup on every request.

It is also the only cache of segment bytes across scans by default: VortexOpenOptions falls back to NoOpSegmentCache. Removing it should therefore show up in cached-mode random access, where repeated takes on an open file currently hit it. SegmentCache remains the explicit way to keep segments across scans.

Changes

  • Removes RetainedSegments, RetainingSegmentSource and vortex-layout/src/scan/v2/retain.rs. Scans read through the file's segment source directly.
  • Removes the chunks_sharing_a_segment_read_it_once test, which tested retention.
  • Tests that turned retention off with ScanCache::with_retained_bytes(0) now use ScanCache::default().

Checks run: cargo clippy -p vortex-layout --all-targets --all-features -- -D warnings (clean), cargo test -p vortex-layout --lib (804 passed).

API Changes

Removes the public ScanCache::with_retained_bytes. Nothing else in the workspace or the bindings calls it.


Generated by Claude Code

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant