Closed Bug 1369263 Opened 3 years ago Closed 2 years ago

Offload all MediaCache activities to another thread

Categories

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

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: jwwang, Unassigned)

References

Details

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.
See Also: → 1361625
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.
Blocks: 1361625
See Also: 1361625
Depends on: 1371200
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.
Depends on: 1373160
Depends on: 1411504
Depends on: 1411803
Depends on: 1411805
Depends on: 1411808
Depends on: 1412181
Depends on: 1412187
Depends on: 1412188
Depends on: 1412204
Depends on: 1412205
Depends on: 1412737
Depends on: 1413483
Depends on: 1413484
Depends on: 1415090
Depends on: 1415438
Depends on: 1415461
Depends on: 1415766
Depends on: 1415069
Depends on: 1416084
Depends on: 1416643
Depends on: 1417305
Depends on: 1417774
Depends on: 1418213
Depends on: 1418219
Depends on: 1418917
Depends on: 1418918
Depends on: 1420016
Depends on: 1419668
Depends on: 1419666
Depends on: 1420324
Depends on: 1420798
Depends on: 1420819
Depends on: 1420828
Depends on: 1421134
Depends on: 1421861
Depends on: 1421864
Depends on: 1421875
Depends on: 1421179
Depends on: 1422657
Depends on: 1422677
Depends on: 1423465
Depends on: 1423482
Depends on: 1424973
Depends on: 1425170
Depends on: 1426056
Depends on: 1426061
Depends on: 1426578
Depends on: 1427666
Depends on: 1427667
Depends on: 1427699
Depends on: 1427931
Depends on: 1428242
Depends on: 1428688
No longer depends on: 1428688
No longer depends on: 1420828
No longer depends on: 1420324
Depends on: 1428951
Depends on: 1429009
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.