perf(subtitles): publish the overlay cue array once per drain tick - #273
Merged
Conversation
$subtitleCues was published once per DECODED PACKET. applyEventMutations takes the channel's array inout and @published exposes get/set with no _modify, so every event both copy-on-wrote the array and republished it. Each publication carries the whole cumulative store, and a snapshot does not say which elements are new, so every consumer walked all of it: O(n) per packet, O(n^2) per drain window. On a typeset ASS track edde746 measured 104 publications and 608,608 cue visits per second in one consumer, with add=0, and 43 ms/s of host time spent on walks that found nothing. The tick now binds the channel's array once, applies the whole batch to it and writes it back once, and only when the batch changed something: an event that decodes but resolves to a cue the store already holds, or a trim matching no open window, no longer costs a publication. The mutation helpers report change up the chain for that. Retention pruning moves out of the per-event path to once per batch, and the retained-store insert finds same-start cues over the equal-start run by binary search instead of scanning the whole array (the dedupe and image-replace keys both require an exact startTime match, and the store is start-sorted). Two further items from the same report: The decode loop is now bounded per tick. The window is bounded in seconds of content and never in packets, so its size was set by the file's density while the loop ran synchronously on the main actor. The bound falls on a PTS boundary: the cursor is a bare PTS advanced by lastDecodedPts.nextUp, so a cut inside a same-PTS run would SKIP its remainder instead of resuming it, and dense ASS deliberately keeps hundreds of distinct payloads on one timestamp. The OCR worker's existing prefix() cap gets the same correction. A capped batch also defers reconstruction finalization, whose contract is "after the whole window decoded". drainPlan compared the live playhead against the playhead captured at the previous tick's start, so any tick longer than the 2.5 s jump threshold made the next one reset onto a fresh disjoint window: a positive feedback loop, since the reset window is the expensive one. Forward drift is now forgiven up to the elapsed wall time; backward drift is not, because playback never moves the playhead backwards. Reported by @edde746 (#271). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HX5zV3Fcf7Nzq4DYGdXvQE
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.
Fixes the three items in #271, all inside the overlay drain tick.
One publication per tick, not one per packet
$subtitleCueswas published once per decoded packet.applyEventMutationstakes the channel's array
inoutand@Publishedexposes get/set with no_modify, so every event both copy-on-wrote the array and republished it. Eachpublication carries the whole cumulative store, and a snapshot does not say
which elements are new, so every consumer walked all of it: O(n) per packet,
O(n²) per drain window. @edde746 measured 104 publications and 608,608 cue
visits per second on a typeset ASS track,
add=0, 43 ms/s of host time spenton walks that found nothing.
The tick binds the channel's array once, applies the whole batch to it, and
writes it back once, only if the batch changed something. An event that decodes
but resolves to a cue the store already holds, or a trim matching no open
window, no longer costs a publication. Retention pruning moves out of the
per-event path to once per batch, and the retained-store insert finds same-start
cues over the equal-start run by binary search instead of scanning the whole
array (both the dedupe and image-replace keys require an exact
startTimematch, and the store is start-sorted).
The batch is bounded, on a PTS boundary
The drain window is bounded in seconds of content (backscan + lead) and never in
packets, so its size was set by the file's subtitle density while the loop ran
synchronously on the main actor with no suspension point.
The bound has to fall on a PTS boundary, as #271 points out: the cursor is a
bare PTS advanced by
lastDecodedPts.nextUp, so a cut inside a same-PTS runwould skip its remainder instead of resuming it, and dense ASS deliberately
keeps hundreds of distinct payloads on one timestamp.
batchEndextends the capforward to the end of the run it lands in; a single run longer than the cap
decodes whole. The OCR worker's existing
prefix()cap gets the samecorrection, since a container that splits a display set across packets shares a
PTS there too.
A capped batch also defers reconstruction finalization, whose contract is "after
the whole window decoded": the successor composition may sit in the remainder.
A slow tick is not a seek
drainPlancompared the live playhead against the playhead captured at theprevious tick's start, so any tick longer than the 2.5 s jump threshold made
the next one reset onto a fresh, disjoint window: a positive feedback loop,
since the reset window is the expensive one. Forward drift is now forgiven up to
the elapsed wall time. Backward drift is not, because playback never moves the
playhead backwards, and re-reading
sourceTimeat tick end (the other option inthe report) would swallow a seek that landed mid-tick, whose only detector this
is.
Verification
Issue271DrainPublicationTests).swift build -Xswiftc -strict-concurrency=completeclean.Device verification of the publication rate on the reporter's ASS sample is still open.
Reported and measured by @edde746.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HX5zV3Fcf7Nzq4DYGdXvQE