fix: cap playlist helpers at one per visit instead of one per batch - #678
Merged
Conversation
Standing down our own auto-load cascade settled the open question from the last PR, and the answer was the one that meant my attribution was wrong: autoload_skipped x3 our cascade did stand down keep-one.register x3 the same three helpers appeared anyway YouTube's viewport refill drives the refetches, not schedulePlaylistAutoLoad. With filtering leaving a single tile on a screen that fits four or five rows, the list is starved and YouTube pulls the whole playlist unaided — 780ms flat: 18:44:00.675 batch collect starts 18:44:00.693 helper 1 (initial response) 18:44:01.022 continuation -> helper 2 18:44:01.239 continuation -> helper 3 18:44:01.455 continuation 18:44:03.338 collector's FIRST extra batch, 2.6s late and irrelevant Which also makes the fix clear. A helper exists only so the list is never empty — an empty list is what makes YouTube TV reload the page. It does not have to be a NEW one each batch, and the measurement above shows the extra ones buy nothing: YouTube keeps loading whether or not we add them. Every one it does add is stranded permanently in the virtual list's data model, unreachable on Tizen 5.0 (654577e), so it stays a blank navigable slot for the rest of the visit. That is the four-slots-and-four-presses symptom. So once a playlist has a helper, later all-watched batches return [] and reuse it. Four helpers become one, and that one is retired and DOM-removed by the existing clearPlaylistHelperVideoIdSet path when the final (!hasContinuation) batch lands. Guarded two ways, because the failure mode of getting this wrong is an empty list and a page reload: - Reuse is tied to the location hash the helper was created under, so a helper stranded by navigating away mid-load cannot suppress helper creation on a different playlist. - A page-level playlist response resets the cap, since a freshly rendered list cannot contain an earlier visit's helper. This also clears the stale retired ids behind the page-restart deadlock 654577e described. Note the pacing delay is now doing nothing useful: this run had zero fetch failures, and it is what puts the collector 2.6s behind a race that is over in 780ms. Left alone here to keep this change to one thing.
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.
Your hypothesis was right. Standing down our own cascade settled it:
YouTube's viewport refill drives the refetches, not
schedulePlaylistAutoLoad. Filtering leaves one tile on a screen that fits 4–5 rows, so the list is starved and YouTube pulls the whole playlist unaided in 780ms:The fix that follows: a helper exists only so the list is never empty (an empty list is what makes YouTube TV reload the page). It doesn't have to be a new one per batch — and the measurement shows the extras buy nothing, since YouTube keeps loading regardless. Each extra is stranded permanently in the virtual list's data model (unreachable on Tizen 5.0,
654577e), i.e. a blank navigable slot for the rest of the visit.So later all-watched batches now return
[]and reuse the existing helper. Four become one, and that one is retired and DOM-removed by the existing path when the final!hasContinuationbatch lands.Guarded twice, since getting it wrong means an empty list and a page reload: reuse is tied to the location hash the helper was created under, and a page-level playlist response resets the cap (which also clears the stale retired ids behind the page-restart deadlock
654577edescribed).Separately noted: the 2.5s pacing delay is now dead weight — zero fetch failures this run, and it's what puts the collector 2.6s behind a 780ms race. Left for its own change.