Skip to content

Design observable IndexedReader caching and hinted prefetch - #21

Draft
rob-p wants to merge 2 commits into
mainfrom
indexed-reader-prefetch
Draft

Design observable IndexedReader caching and hinted prefetch#21
rob-p wants to merge 2 commits into
mainfrom
indexed-reader-prefetch

Conversation

@rob-p

@rob-p rob-p commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

This is the focused, design-first successor to the decoded-window cache and seek-prefetch work in #5.

The feature is warranted for repeated, overlapping, and caller-known random ranges, but not with #5's automatic two-window policy or ad-hoc thread creation. This draft specifies an explicit decoded cache and caller-hinted prefetch with a lazy bounded worker pool, dynamic zero-to-budget control, usefulness telemetry, and unchanged zero-thread defaults.

Key decisions

  • IndexedReader::new remains cache-free and creates no background thread.
  • Complete linear indexed decoding remains the job of reader_from_index.
  • Prefetch is caller-hinted rather than guessed from the current cursor.
  • A dedicated handle reports live/busy workers, queue/cache bounds, and whether prefetched chunks were used or evicted unused.
  • A runtime worker limit of zero disables advisory work and retires idle workers.
  • Canonically aligned decoded chunks deduplicate overlapping range requests.
  • The current partial-verification boundary for interior checkpoints is preserved exactly.
  • No new dependency or unsafe block is planned.

Gate before implementation

The document deliberately stages synchronous caching before worker prefetch. Work stops if repeated-range caching does not produce a representative win without changing the default path. Background prefetch must then improve at least one real multi-range trace while respecting worker/RSS bounds and leaving disabled behavior statistically unchanged.

Review focus

  • Is caller-hinted prefetch the right initial contract, or is a batch range API preferable?
  • Are dedicated indexed-reader telemetry types clearer than widening DecoderStats semantics?
  • Are the canonical chunk, cancellation, cache-admission, and into_inner ownership rules sufficient?

This PR currently contains the complete design and implementation/validation plan. Implementation will follow on this branch after design review.

@rob-p rob-p mentioned this pull request Aug 4, 2026
@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor

Taking your three review questions in order.

Caller-hinted prefetch versus a batch range API. I would start with the batch API and treat hinting as the streaming special case, not the reverse. A batch call states the whole working set, so the implementation can order reads by compressed offset, deduplicate overlapping canonical chunks, and size the pool against known work rather than guessing from a queue of advisory hints. It also makes the usefulness telemetry trivially interpretable, since requested and delivered are the same set. Hinting only wins when the caller genuinely cannot know the next range, which for the motivating case of a reader walking a list of records is rarely true. If both land eventually, the batch API is the one whose contract does not need a usefulness metric to explain itself.

Dedicated telemetry types versus widening DecoderStats. Dedicated, clearly. DecoderStats describes a decode in flight with a worker population and a pressure classification; a cache reports hit rate, residency, and eviction, and its worker pool can legitimately be empty for the object's whole life. Widening the existing type would give every field a "meaningful only when" caveat, which is how a telemetry struct becomes unreadable. The cost of a second type is one more thing to document, and that is cheaper than overloading a type users already read for a different purpose.

Canonical chunks, cancellation, admission, into_inner. The rules look sufficient for the design stage. The one I would want pinned before implementation is into_inner while prefetch is in flight: the document says ownership returns, but not whether it blocks for outstanding workers or cancels and discards. Both are defensible; the difference is observable, since one can block for the length of a queued decode. Worth stating in the same sentence that promises the source back.

One thing I would add to the gate rather than to the design. The document stages caching before prefetch and stops if caching does not pay, which is right. I would also require the disabled path to be measured, not assumed, in the same run that measures the enabled one. IndexedReader::new staying cache-free and thread-free is the property most likely to erode quietly once a cache exists in the same type, and it is the property most users depend on without knowing it.

On #22, I reviewed and built it and left detailed comments there. The one cross-cutting note is that its flaky neighbour final_reader_handoff_reports_consumer_backpressure made that review harder than it should have been, so I opened #24 to fix the assertion. If this branch grows a worker pool with its own retirement behaviour, the same class of test will appear here, and the lesson from #24 transfers: assert the admission ceiling the runtime promises, not the operating-system thread count, which cannot drop while a worker holds output nobody has read.

@rob-p

rob-p commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Release decision: this work is explicitly deferred to 0.3.0.

Version 0.2.0 will ship the existing verified IndexedReader, explicit index
construction, interoperable index formats, and strict full-stream parallel
decode through an index. It will not add decoded-window caching, background
prefetch workers, or any change to the cache-free and thread-free default
IndexedReader::new path.

The design remains useful and this draft should stay open as the 0.3.0 design
record. Before implementation, it should be updated onto the then-current
main and incorporate the review direction already recorded here: evaluate a
batch-range API before advisory hints, keep cache/prefetch telemetry separate
from DecoderStats, specify whether into_inner cancels or waits for in-flight
prefetch, and benchmark the disabled path in the same run as the enabled path.

This deferral is intentional and is not a blocker for 0.2.0.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants