1369263 - Offload all MediaCache activities to another thread

Status: RESOLVED FIXED

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

Description

[JW Wang [:jwwang] [:jw_wang]]

Since now Necko supports off main thread data callbacks, it should be possible to run the entire MediaCache code off the main thread. This should fix MediaCache-related main-thread jank once al for all.

This bug is to discuss the feasibility and difficulty that might appear ahead of the road.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

As I was looking at bug 1361625, I had a go a just moving Update() to a separate thread, which ended horribly -- as could have been expected:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a484dee774af281c8ee36b0aa45aaad37262940

A couple of lessons:
- Lots of MediaCache member variables are accessed without locks from the main thread (because they're only supposed to be written there).
But if you can move *all* activities to another thread, this shouldn't be a problem.

- During Update() on a non-main thread, calling client callbacks can easily crash and burn:
It could be releasing the MediaCache on that thread; or in the callback itself it assumes it runs on the main thread (e.g., when accessing DOM objects), etc.
So you would need to dispatch callbacks back to the main thread, or fix them so they can run elsewhere.


So it's not an easy exercise, but would probably be worth it, as it would remove quite a bit of heavy code from the main thread.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

JW, as you can see from bug 1361625's dependencies, I've already implemented a few features that should relieve the pressure on the main thread: (with help from :cpearce)
- Bug 1365534: Move FileBlockCache init/close file IOs off main thread
- Bug 1365538: Prioritize reads in FileBlockCache
- Bug 1368837: Add lockless cache to MediaResourceIndex
From my own test logging, I noticed the number of main-thread >10ms blockages dropping by 1000x! :-)
I'm monitoring BHR results to see if these help in the field.

And now, based on the following data:
- bug 1366929: MEDIACACHE_WATERMARK_KB telemetry, showing >90% of MediaCache instances don't go over 8MB.
- bug 1369538: MEDIACACHESTREAM_LENGTH_KB telemetry, showing >90% of MediaCacheStream instances don't go over 8MB.
Chris suggested that when we start downloading "small-enough" resources, we could just bypass the shared MediaCache, and keep the whole resource in memory. So the resource itself will have zero file IOs; and it won't impact the shared MediaCache that will still take care of bigger resources.
I'm working on it right now (I will open a bug soon), so I'm modifying MediaCache quite heavily, and I would really appreciate if you could delay work on this bug for a little while, thank you.

I'm hopeful all this work might make this bug here not as urgent. (But it would still be nice to eventually do, of course.)

Please contact me if you have questions.

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

All dependencies are resolved.