Don't prevent MemoryBlockCache post-init buffer growth

RESOLVED FIXED in Firefox 56

Status

()

Core
Audio/Video: Playback
P1
normal
RESOLVED FIXED
4 months ago
27 days ago

People

(Reporter: gerald, Assigned: gerald)

Tracking

(Blocks: 1 bug)

56 Branch
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

4 months ago
MemoryBlockCache records the following telemetry:
https://telemetry.mozilla.org/new-pipeline/dist.html#!compare=architecture&measure=MEMORYBLOCKCACHE_ERRORS

Results so far show that in some <1% cases we hit 4=WriteBlockCannotGrow, which means we needed to grow the buffer (because the file was actually larger than initially hinted by the HTML content-length) but hit the allowed combined limit of all MemoryBlockCache. This is only visible on 32-bit systems so far, probably because they have the smallest amount of memory.

Because of this, we are preventing further playback of these files, which is not acceptable.

One easy solution would be to just disable MemoryBlockCache on 32-bit systems.
But I'm afraid this issue will appear on 64-bit systems once we hit more populous beta and then release versions.
So I think we should have a proper handling for these.

I believe it should be possible for the MemoryBlockCache to instantiate a FileBlockCache, move all of its data there (I/Os are done in a separate thread anyway), and then channel subsequent calls directly to the FileBlockCache.
(Assignee)

Comment 1

4 months ago
I've just realized that a memory-backed MediaCache could limit how much it uses its MemoryBlockCache, by tailoring GetMaxBlocks(). Much easier!
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 4

4 months ago
Changing bug title to avoid future confusion, as we're not really handling MemoryBlockCache growth failures, but just making them not happen anymore (apart from OOM).
Summary: Handle failure to grow MemoryBlockCache → Don't prevent MemoryBlockCache post-init buffer growth
Priority: -- → P1

Comment 5

3 months ago
mozreview-review
Comment on attachment 8884680 [details]
Bug 1379091 - Let block cache tell MediaCache its block use limit -

https://reviewboard.mozilla.org/r/155538/#review160982
Attachment #8884680 - Flags: review?(cpearce) → review+

Comment 6

3 months ago
mozreview-review
Comment on attachment 8884681 [details]
Bug 1379091 - Don't prevent MemoryBlockCache overuse -

https://reviewboard.mozilla.org/r/155540/#review160984
Attachment #8884681 - Flags: review?(cpearce) → review+

Comment 7

3 months ago
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/82220cb42a60
Let block cache tell MediaCache its block use limit - r=cpearce
https://hg.mozilla.org/integration/autoland/rev/a7e5cdf5c07f
Don't prevent MemoryBlockCache overuse - r=cpearce

Comment 8

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/82220cb42a60
https://hg.mozilla.org/mozilla-central/rev/a7e5cdf5c07f
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1397528
No longer depends on: 1397528
You need to log in before you can comment on or make changes to this bug.