Closed Bug 1369263 Opened 3 years ago Closed 2 years ago
Offload all Media
Cache activities to another thread
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.
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.
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.
3 years ago
Priority: -- → P3
3 years ago
Depends on: 1382978
All dependencies are resolved.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.