1057196 - Make MediaCache cache size bigger for mochitests

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

I noticed that the behaviour of the new MP4Reader is very bad in mochitest on Windows in debug builds, worse than the existing WMFReader. Specifically I was testing with gizmo.mp4 in test_bug495145.html.

After diving into it, I discovered that the MediaChannelResource's stats think we're getting data from httpd.js at about 20KB/s, and we're frequently stopping to buffer data. Looking a bit closer, I realized that the new MP4 demuxer is seeking all over the resource while we're trying to load it, and coupled with a small media cache size, we're ending up having to do a lot of small HTTP requests and blocking in the readers waiting for the requests to come in.

Making the media cache bigger makes the MP4Reader perform much better:

With MP4Reader enabled and media.cache_size=100 test_bug495145 completes in 3m43.041s
with media.cache_size=1000, completes in 2m5.976s. (Note this includes non-MP4 tests in this test as well).

In a more general case on my laptop, a full content/media/test run takes ~88 minutes with media.cache_size=100 (100KB), and ~65 minutes with media.cache_size=1000. With media.cache_size=100000 I also measured ~65 minutes. (This is on Windows in debug builds, mochitest in debug builds is very slow on Windows).

We should make the media cache bigger. We haven't found any bugs in the media cache for a while. 1MB is a good value, as setting it to 100MB didn't perform better, and 1MB is large enough to fix 2 x gizmo.mp4 inside with some slack.

I expect that making the media cache bigger will make our tests time out less in general, as there will be less waiting around for network requests.

I have not noticed this being a problem when using the MP4Reader outside of mochitests, I tested on a low end Atom Z2760 Win8 tablet, and it works fine on HD video. Presumably the media cache is large enough in normal builds that the extra seeking the MP4Reader does is not a problem.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Increase MediaCache in tests from 100KB to 1MB

We should at some stage add a test specifically with a lower cache size to stress test the mediacache.

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8477176 [details] [diff] [review]
Patch: Increase MediaCache in tests from 100KB to 1MB

Review of attachment 8477176 [details] [diff] [review]:
-----------------------------------------------------------------

Risk vs reward seems worth it, let's do it.

Comment 3

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

I also saw pref("media.cache_size", 4096) in b2g/app/b2g.js and pref("media.cache_size", 512000) in modules/libpref/init/all.js. Which one will take effect in the mochitests?

Generally speaking, we want the test environment as close to the practical environment as possible. Otherwise we might hide issues met by the end user.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to JW Wang [:jwwang] from comment #3)
> I also saw pref("media.cache_size", 4096) in b2g/app/b2g.js and
> pref("media.cache_size", 512000) in modules/libpref/init/all.js. Which one
> will take effect in the mochitests?

Those are overridden by user_pref. I believe the precedence from least to greatest goes all.js, app-specific prefs (b2g.js, firefox.js, etc), and for mochitest then prefs_general.js.

> Generally speaking, we want the test environment as close to the practical
> environment as possible. Otherwise we might hide issues met by the end user.

I agree, but we also need the media.cache_size to be small enough that we exercise the eviction code, at least a bit. Also, we're testing very small media, whereas the typical video people watch online is much bigger. So arguably we're already not testing typical use cases.

Comment 5

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

Sounds good. Lets go for it.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4af6581e7c63

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4af6581e7c63