1974237 - Optimize the performance for `TextTrackCueList::AddCue()`

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[eduter]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:139.0) Gecko/20100101 Firefox/139.0

Steps to reproduce:

This is more noticeable on text tracks with many cues, like the one in this manifest, containing tens of thousands of them:

https://ed14.cdn.svt.se/d0/world/20250623/9ba84fdc-325a-426b-896f-6d0a14a29226/dash-full.mpd

Here's a link to play it in the Dash.js reference player:
http://localhost:3000/samples/dash-if-reference-player/index.html?mpd=https%3A%2F%2Fed14.cdn.svt.se%2Fd0%2Fworld%2F20250623%2F9ba84fdc-325a-426b-896f-6d0a14a29226%2Fdash-full.mpd&autoLoad=true&muted=true+&debug.logLevel=5&streaming.capabilities.supportedEssentialProperties.0.schemeIdUri=urn%3Advb%3Adash%3Afontdownload%3A2014&streaming.capabilities.supportedEssentialProperties.1.schemeIdUri=urn%3Ampeg%3AmpegB%3Acicp%3AColourPrimaries&streaming.capabilities.supportedEssentialProperties.2.schemeIdUri=urn%3Ampeg%3Adash%3Aurlparam%3A2014&streaming.capabilities.supportedEssentialProperties.3.schemeIdUri=urn%3Ampeg%3Adash%3Aurlparam%3A2016&streaming.capabilities.supportedEssentialProperties.4.schemeIdUri=urn%3Ampeg%3AmpegB%3Acicp%3AMatrixCoefficients&streaming.capabilities.supportedEssentialProperties.5.schemeIdUri=urn%3Ampeg%3AmpegB%3Acicp%3ATransferCharacteristics&streaming.capabilities.supportedEssentialProperties.6.schemeIdUri=http%3A%2F%2Fdashif.org%2Fthumbnail_tile&streaming.capabilities.supportedEssentialProperties.7.schemeIdUri=http%3A%2F%2Fdashif.org%2Fguidelines%2Fthumbnail_tile&streaming.delay.liveDelayFragmentCount=NaN&streaming.delay.liveDelay=NaN&streaming.buffer.initialBufferLevel=NaN&streaming.liveCatchup.maxDrift=NaN&streaming.liveCatchup.playbackRate.min=NaN&streaming.liveCatchup.playbackRate.max=NaN

While playing it, run this in the console:

(()=>{
const track = $('video')[0].textTracks[0]
const newMode = track.mode === 'showing' ? 'hidden' : 'showing'
const before = performance.now()
track.mode = newMode
const elapsedTime = performance.now() - before
const cues = track.cues?.length ?? 0;

console.log(Setting text track mode to ${newMode} took ${elapsedTime}ms (${cues} cues));
})()

Actual results:

The main thread got blocked for over a minute and the snippet above printed:

Setting text track mode to hidden took 78990ms (38780 cues)

Expected results:

The browser should have remained responsive, without any noticeable lag. Doing exactly the same in Chrome, I got this:

Setting text track mode to hidden took 8.599999904632568ms (38780 cues)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video: Playback' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Andreas Pehrson [:pehrsons]]

Note the public link to the dash reference player is here.

And the code to run in a code block:

(()=>{
  const track = $('video')[0].textTracks[0]
  const newMode = track.mode === 'showing' ? 'hidden' : 'showing'
  const before = performance.now()
  track.mode = newMode
  const elapsedTime = performance.now() - before
  const cues = track.cues?.length ?? 0;

  console.log(`Setting text track mode to ${newMode} took ${elapsedTime}ms (${cues} cues)`);
})()

On my Macbook Pro (M2 Pro) it takes ~37 seconds (Setting text track mode to hidden took 36877ms (38780 cues)), which is certainly enough to run out of buffered media and stall playback.

Comment 3

[Andreas Pehrson [:pehrsons]]

Here's a profile of just a 1-second slice with only WebVTT:5 logging during loading of the video. I didn't get to run the provided script in the console, but you get an idea of the performance issues here.

Comment 4

[Andreas Pehrson [:pehrsons]]

Alastor, do you have some time to take a look?

Comment 5

[Andreas Pehrson [:pehrsons]]

Here's another profile, without logging (the overhead becomes too much). This one covers the full page load and the script.

Yes, we do seem pretty slow overall, intensely so for setting track mode. Without having looked at the code I wonder why we are iterating over all cues doing something, rather than just affect the cues that cover the current time, and perhaps the next cue after that. TL;DR setting the mode takes 39s, of which ~36s is made up of various media::TimeUnit operations.

Re loading, which is outside the scope of this bug, the deeper issue seems to be that dash.js doesn't let the event loop run until it has added all cues for the 7h video? IMHO it could just stream them, making sure to have added all cues from currentTime and a few seconds ahead. Or follow the duration of buffered media data. I mean -- 7h worth of cues added and then the user changes language! What a waste.

Comment 6

[Andreas Pehrson [:pehrsons]]

Looking a bit into the track mode setter, the problem is:

• For each cue (~39k of them) in the (unsorted) cue list:
  • Notify the media element it was added or removed, depending on mode.
  • Add or Remove it from the list of active cues, depending on mode.
  • What is the list of active cues? A time-sorted (media::TimeUnit) list! And we add elements one by one! Geeee

And of course, if dash.js kept, say, 100 cues in the track at any given time instead of 39k, this wouldn't be a user-observable problem.

edit: nope, the list of active cues use double for sorting, so the TimeUnit operations come in elsewhere.

Comment 7

[Alastor Wu [:alwu]]

I can take some time to look into this tomorrow—keep my NI.

When I clicked the player link in comment 2, I saw an error after hitting “Load”:
https://ed14.cdn.svt.se/d0/world/20250623/9ba84fdc-325a-426b-896f-6d0a14a29226/dash-full.mpd is not available.
Did I do something wrong?

Also, how does the performance compare on Chrome? Thanks.

Comment 8

[Andreas Pehrson [:pehrsons]]

Nothing wrong, it should load automatically on document load.

Reporter, is this content region blocked? If so, do you have something accessible from north america?

Per comment 0 we're a factor ~10x ~10000x slower than Chrome on setting the mode. That's still multiple seconds worth of jank in Chrome on a fast machine.

edit: oh, it was millis!

Comment 9

[Andreas Pehrson [:pehrsons]]

There is not much use of TimeUnit in webvtt, but there's this in TextTrack::GetCurrentCuesAndOtherCues, which does TimeUnit operations on every cue in the list, and is only called from TextTrackManager::TimeMarchesOn.

TextTrackManager::TimeMarchesOn happens (by spec?) to be called synchronously from both SetMode and AddCue (through e.g. NotifyCueAdded, i.e. on every cue). Yeah that's quadratic.

Comment 10

[Mayank Bansal]

Bug 1951071 may be related (and has a minimal artificial testcase)

Comment 11

[eduter]

The content should not be blocked by region, but I tried with a VPN from the US and also got that error, for some reason. When I used our player (also using Dash.js) instead of the reference player, it worked from the US: https://videoplayer-static.app.borealis.svt.se/demo/master/?videoId=%22ed61zDr%22

About the performance on Chrome, on my machine it was a factor of ~10000x. On Chrome it takes ~8ms, while on Firefox, ~78000ms.

Comment 12

[Andreas Pehrson [:pehrsons]]

In a quick experiment I put all the TimeMarchesOn(), MaybeRunTimeMarchesOn() and UpdateCueDisplay() calls behind a watch manager and got
Setting text track mode to hidden took 255ms (38780 cues), and
Setting text track mode to showing took 237ms (38780 cues), for two rounds of the console script, respectively.

Note this was a local, optimized, debug build, so it may be faster yet in non-debug/release. Some irrelevant webrtc dirs were compiled unoptimized, which I believe is why the profiler compiles about optimizations.

See the profile here.

I haven't tried running the test suite.

Comment 13

[Andreas Pehrson [:pehrsons]]

Ok, the WPT WebVTT suite is happy locally.

Comment 14

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1974237 - Put heavy and hot TextTrackManager methods behind a WatchManager for lazy once-per-task execution. r?alwu

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c9df26d00c81
https://hg.mozilla.org/integration/autoland/rev/dc5ce2871054
Put heavy and hot TextTrackManager methods behind a WatchManager for lazy once-per-task execution. r=alwu

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/afe015e726c7
https://hg.mozilla.org/integration/autoland/rev/79b0be0dcc92
Revert "Bug 1974237 - Put heavy and hot TextTrackManager methods behind a WatchManager for lazy once-per-task execution. r=alwu" for causing WPT failures @activeCues.html

Comment 17

[pstanciu]

Backed out for causing WPT failures @activeCues.html

• https://hg.mozilla.org/integration/autoland/rev/79b0be0dcc92a1943ce1101ed4d40ef07011a039
• Push with failures
• Failure Log

Comment 18

[Alastor Wu [:alwu]]

Add NI to check wpt failure later.

Comment 19

[Alastor Wu [:alwu]]

Since D255294 is not compliant with the spec, the "time marches on" algorithm must run immediately after any of the relevant conditions change. To address performance concerns, I optimized the algorithm itself, achieving approximately 400x improvement in Bug 1975580.

However, the change to use WatchManager for cue display in D255294 should still work. We can try that in this bug as well.

Comment 20

[Alastor Wu [:alwu]]

After fixing bug 1975580, this should be a trivial issue.

Comment 21

[Alastor Wu [:alwu]]

The performance bottleneck was in TextTrackCueList::AddCue(). After optimizing that function, I'm able to achieve comparable performance—from ~200ms to ~5ms—in my local debug build.

Comment 22

[Alastor Wu [:alwu]]

Attachment: Bug 1974237 - part1 : remove unused methods.

Comment 23

[Alastor Wu [:alwu]]

Attachment: Bug 1974237 - part2 : use nsTHashSet to perform O(1) search in order to improve the performance.

In a test scenario with approximately 40000 cues, the speed improvement is
dropping from around 200ms to about 5ms.

Comment 24

[Alastor Wu [:alwu]]

Attachment: Bug 1974237 - part3 : fix clang warnings.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/bc75bd8cdcfa
https://hg.mozilla.org/integration/autoland/rev/331337e5fdb8
part1 : remove unused methods. r=media-playback-reviewers,padenot
https://github.com/mozilla-firefox/firefox/commit/0cf99a4728a9
https://hg.mozilla.org/integration/autoland/rev/6d41b0e9c444
part2 : use nsTHashSet to perform O(1) search in order to improve the performance. r=media-playback-reviewers,padenot
https://github.com/mozilla-firefox/firefox/commit/dab6df391f5d
https://hg.mozilla.org/integration/autoland/rev/31ddd392b913
part3 : fix clang warnings. r=media-playback-reviewers,padenot

Comment 26

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/331337e5fdb8
https://hg.mozilla.org/mozilla-central/rev/6d41b0e9c444
https://hg.mozilla.org/mozilla-central/rev/31ddd392b913