Closed Bug 1474883 Opened Last year Closed Last year

Memory leak due to animated frames

Categories

(Core :: Graphics: Layers, defect, P3)

Unspecified
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- fixed
firefox61 --- wontfix
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: ce-ce-mel, Assigned: jnicol)

References

()

Details

(Keywords: memory-leak, Whiteboard: [MemShrink:P1])

Attachments

(3 files)

Attached file memory-report.json.gz
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180704003137

Steps to reproduce:

This happened on two different computers (both Firefox 61.0.1 + Windows 1803), using a new profile. It does not happen in safe mode.

Step to reproduce:

1. Visit https://www.proximus.be/en/id_cr_roaming/personal/products/mobilus/mobile-and-roaming-options/international-and-roaming/international-and-roaming-rates.html
2. Wait a few seconds without changing tab 

NB: change tab, close it or remove the animated block (the globe on the right side of the page) before it fills your RAM completely

NB2: there is a memory report attached. However, since defocusing the tab result of memory freeing, I do not know if it the file is relevant.


Actual results:

Memory usage keeps growing rapidly (in my case, 15 s was enough to allocate 12 GB)


Expected results:

A decent memory allocation
Summary: Memory leak due to animated element → Memory leak due to animated frames
I managed to reproduce this issue on Firefox 61.0.1, 62.0b7 and Nightly 63.0a1 (2018-07-15), Ubuntu 16.04, Max OS and Windows 10.
Status: UNCONFIRMED → NEW
Component: Untriaged → ImageLib
Ever confirmed: true
OS: Unspecified → All
Product: Firefox → Core
See Also: → 1303736
Version: 61 Branch → Trunk
Whiteboard: [MemShrink]
A tip for measuring image related memory with about:memory, open a new window for about:memory, that way the problematic tab can stay as a foreground tab and we shouldn't get rid of an image memory from it.

I suspect this is actually graphics. The animating thing on that page isn't an animated image. I am able to reproduce on Windows but not Mac (also points to graphics as almost everything imagelib is crossplatform). When I reproduce nothing much seems to show up in about memory (including images which are quite well measured by about:memory). Graphics drivers and such are less well measured by about:memory.
Component: ImageLib → Graphics
I cannot reproduce on Linux, but certainly can on Windows, either with or without advanced layers. Turning off acceleration altogether fixes it, which explains the reporter not experiencing it in safe mode.

Will dig deeper.
Assignee: nobody → jnicol
Component: Graphics → Graphics: Layers
OS: All → Windows
Priority: -- → P3
I've bisected this, and the bad change is a change I made - bug 1339578. This introduces a minimum size for creating active layers for animated transforms. Setting layers.min-active-layer-size = 0 in about:config stops the leak.

I do sometimes question whether that was a good change - like all layerization heuristics sometimes it's good and sometimes it's bad. But in any case - there is an underlying bug. We're leaking memory somewhere in the inactive code path.
Whiteboard: [MemShrink] → [MemShrink:P1]
Attached file testcase.html
Each of the animated icons is actually a glyph rendered using a special font, with an animated transform. What's happening here is that with inactive layers we render them all in to a single active layer, which means there are only two d2d render targets (a backbuffer and frontbuffer) which live forever. Furthermore, as they are rasterized directly in to the page's main painted layer, they are rasterized with rotational transforms (which keep changing values).

This is causing D2D's glyph cache to grow very large, as it will have a different version of each glyph for each rotation.

When using active layers, the layers and therefore the render targets only live a short while. And more importantly, the glyphs are rasterized un-rotated in to their layer buffer, then composited with a rotation - so the glyph cache does not grow large.

Attached is a reduced test case which reproduces the bug.

Even with this explanation, however, it still surprises me quite how much memory it is using.

As for how to fix this: the glyph cache seems to be pruned when calling EndDraw/BeginDraw. We do this in DrawTargetD2D1::FlushInternal. We deliberately avoid doing this as much as possible, however, as it is costly. So we should do it every so often when rendering lots of glpyhs, but not too often.
Comment on attachment 8995969 [details]
Bug 1474883 - Ensure D2D glyph cache is pruned after rendering 1000 transformed glyphs.

https://reviewboard.mozilla.org/r/260258/#review267298
Attachment #8995969 - Flags: review?(bas) → review+
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f86485d19c69
Ensure D2D glyph cache is pruned after rendering 1000 transformed glyphs. r=bas
https://hg.mozilla.org/mozilla-central/rev/f86485d19c69
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
This sounds bad. Please nominate for Beta & ESR60 approval when you get a chance.
Comment on attachment 8995969 [details]
Bug 1474883 - Ensure D2D glyph cache is pruned after rendering 1000 transformed glyphs.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Very bad memory leak.
User impact if declined: On affected websites firefox can begin to use gigabytes of memory per a few seconds and keeps increasing until the system becomes unresponsive or it crashes. Not sure how frequently affected websites would occur.
Fix Landed on Version: 63
Risk to taking this patch (and alternatives if risky): Could theoretically have performance impact. Though I didn't notice on talos or my own profiles.
String or UUID changes made by this patch: N/A

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: Very long-standing issue, uncovered by bug 1339578 (which was still over a year ago)
[User impact if declined]: On affected websites firefox can begin to use gigabytes of memory per a few seconds and keeps increasing until the system becomes unresponsive or it crashes. Not sure how frequently affected websites would occur.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Don't think so.
[Why is the change risky/not risky?]: Uses an existing code path to be more conservative with memory usage. Minor performance risk.
[String changes made/needed]: N/A
Flags: needinfo?(jnicol)
Attachment #8995969 - Flags: approval-mozilla-esr60?
Attachment #8995969 - Flags: approval-mozilla-beta?
Comment on attachment 8995969 [details]
Bug 1474883 - Ensure D2D glyph cache is pruned after rendering 1000 transformed glyphs.

Fixes a massive memory leak for Windows users. Approved for 62.0b16 and ESR 60.2.
Attachment #8995969 - Flags: approval-mozilla-esr60?
Attachment #8995969 - Flags: approval-mozilla-esr60+
Attachment #8995969 - Flags: approval-mozilla-beta?
Attachment #8995969 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.