Slice: Add prefetch deduplication and freelist#12949
Draft
zwoop wants to merge 2 commits intoapache:masterfrom
Draft
Slice: Add prefetch deduplication and freelist#12949zwoop wants to merge 2 commits intoapache:masterfrom
zwoop wants to merge 2 commits intoapache:masterfrom
Conversation
Track in-flight prefetches per remap instance using a mutex- protected set to prevent duplicate upstream requests. Recycle BgBlockFetch objects via a freelist under the same mutex. Fire the initial prefetch burst at header parse time so faster clients benefit sooner. Co-Authored-By: Craig Taylor (and research)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the slice plugin’s background prefetching by deduplicating in-flight prefetch requests per remap Config instance and reusing BgBlockFetch objects via a mutex-protected freelist. It also triggers the initial prefetch burst earlier (at server header-parse time) to improve time-to-first-byte for fast clients.
Changes:
- Add
schedule_prefetch()helper and shift initial prefetch scheduling to server response header parse time. - Update
BgBlockFetch::schedule()to accept a URL-derived key and deduplicate viaConfig-owned tracking. - Add
Configprefetch state: mutex + active-key set + freelist, with cleanup inConfigdestructor.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/slice/util.h | Exposes schedule_prefetch() helper. |
| plugins/slice/util.cc | Implements schedule_prefetch() and uses it for sliding-window prefetching. |
| plugins/slice/server.cc | Schedules initial prefetch burst once response headers are parsed. |
| plugins/slice/prefetch.h | Extends BgBlockFetch with config/key state and updated schedule() signature. |
| plugins/slice/prefetch.cc | Implements dedup + freelist release/cleanup and integrates them into bg fetch lifecycle. |
| plugins/slice/Config.h | Adds prefetch dedup/freelist data structures and APIs. |
| plugins/slice/Config.cc | Implements prefetchAcquire() and calls cleanup from Config destructor (plus UNITTEST stubs). |
Guard prefetchCleanup() with m_prefetch_mutex to prevent data race with concurrent prefetchRelease() calls. Add unit tests for prefetchAcquire() dedup semantics. Add missing #include <string_view> to prefetch.h. Null-guard the result of TSUrlStringGet() in schedule_prefetch() before constructing std::string_view.
Contributor
Author
|
Addressed Co-Pilot's concerns. |
Member
|
[approve ci] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Track in-flight prefetches per remap instance using a mutex- protected set to prevent duplicate upstream requests. Recycle BgBlockFetch objects via a freelist under the same mutex. Fire the initial prefetch burst at header parse time so faster clients benefit sooner.
Co-Authored-By: Craig Taylor (who also did all the investigations"