perf: resume collection from YouTube's own batches instead of refetching them - #683
Merged
Conversation
…ing them
The accumulator added over the last two PRs still never completed, and the
log finally shows why it never could:
playlist.continuation.detected itemCount:15 hasContinuation:true
playlist.continuation.detected itemCount:15 hasContinuation:true
playlist.continuation.detected itemCount:15 hasContinuation:true
That is 15 initial + 45 = 60 of 68 items, and every one says there is more
to come. YouTube stops there of its own accord — it never fetches the last
8-item batch unless the user scrolls — so the "no continuation token"
completion signal the accumulator waits for simply never arrives. Hooking
the array-root path was not wrong, there was just no final native batch to
catch.
Meanwhile the collector, started at page load, worked from the INITIAL
token and refetched batches 2-5 from scratch: 53 items, 45 of which were
already on screen, at 2.5s per request. Roughly ten seconds of helper tiles
for eight missing items.
So the two halves are now joined, which is what was actually being asked
for: hold the collector until the native burst goes quiet
(NATIVE_SETTLE_MS, reset by each arriving batch), then start it with the
items already accumulated as its seed and the NEWEST continuation token
YouTube reached. _collectAll already treats plc.contents as its starting
set, so it simply carries on and fetches only what is genuinely missing —
one batch here instead of four.
Also drops BATCH_FETCH_DELAY_MS from 2500ms to 400ms. That figure was
picked while timing was wrongly blamed for the {"error":...} rejections;
the real cause was missing request headers, and nothing has failed since
those were added. With seeding it is usually one fetch, so most of that
2.5s was pure latency.
Expected: native settles ~0.75s, collector fills the gap, cache and reload
land in roughly 2-3s instead of ~10.5s.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Valid finding. This PR replaced adblock.js's direct autoStartCollect() call with scheduleCollectAfterNativeSettles(), so the import was left behind referenced only by a comment. - Removed it from adblock.js's import list. - Unexported autoStartCollect: nothing outside the module calls it any more, it is reached only through the scheduler. - Corrected two doc comments that still described adblock.js invoking it directly on page load, which is no longer how the flow works. No behaviour change.
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
2 participants
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.
This is the merge of the two halves you asked for.
The accumulator never completed, and the log shows it never could:
15 initial + 45 = 60 of 68, and every one says more is coming. YouTube stops there by itself — it never fetches the final 8-item batch unless you scroll — so the "no continuation token" completion signal never arrives. Hooking the array-root path wasn't wrong; there was simply no final native batch to catch.
Meanwhile the collector started at page load from the initial token and refetched batches 2-5 from scratch: 53 items, 45 already on screen, at 2.5s each. Ten seconds of helper tiles to obtain eight missing items.
Now the fast pass feeds the slow one: hold the collector until the native burst goes quiet (reset by each arriving batch), then start it with the accumulated items as its seed and the newest token YouTube reached.
_collectAllalready treatsplc.contentsas its starting set, so it carries on and fetches only what's genuinely missing — one batch instead of four.Also drops the pacing delay 2500ms → 400ms. That number was chosen while timing was wrongly blamed for the
{"error":...}rejections; the real cause was missing headers, and nothing has failed since. With seeding it's usually a single fetch, so most of it was pure latency.Expected: ~2-3s instead of ~10.5s, with
native_settled {seeded:45}then onebatch_fetchedrather than four.