Closed
Bug 1259785
Opened 8 years ago
Closed 8 years ago
memory leak in canvas draw operations (regression)
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: szdy12, Assigned: bas.schouten)
References
Details
(Keywords: regression, testcase, Whiteboard: [MemShrink])
Attachments
(3 files)
4.13 KB,
text/html
|
Details | |
267.35 KB,
text/x-gzip-html
|
Details | |
58 bytes,
text/x-review-board-request
|
jrmuizel
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta-
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 Build ID: 20160322075646 Steps to reproduce: Open the attached html file. Click on "Start" to start a canvas animation. Watch memory usage in Task Manager: Firefox will be using more and more memory over time. Actual results: Firefox is using more and more memory during canvas animation. Expected results: Memory usage should be constant since there are no new memory allocations in the animation loop. This is a regression issue starting from Firefox 45. Firefox 46.0b4 is still leaking memory.
Updated•8 years ago
|
Component: Untriaged → Canvas: 2D
Product: Firefox → Core
Comment 1•8 years ago
|
||
Can reproduce it on Win10x64 with 64bit Nightly. My STR is slightly differen : 1. Open the attachement 2. Click on start 3. Repeatedly rapidly click on "Single Step" for 10-20 seconds. The content-process gets 100% CPU use, and memory keeps on increasing.
Regression range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b4c27a b3cafeef188e84a704cbe72f27419a1eea&tochange=e4944eb37eecb1587c5acec96a14bd7f7d6f de57 Bas Schouten — Bug 1221616: Use ID2D1CommandList instead of a bitmap for temporary D2D drawing. r=jrmuizel Bas, could you check why your patch is leaking memory when drawing on canvas?
Blocks: 1221616
Status: UNCONFIRMED → NEW
status-firefox45:
--- → affected
status-firefox46:
--- → affected
status-firefox47:
--- → affected
status-firefox48:
--- → affected
status-firefox-esr45:
--- → affected
tracking-firefox46:
--- → ?
tracking-firefox47:
--- → ?
tracking-firefox48:
--- → ?
tracking-firefox-esr45:
--- → ?
Component: Canvas: 2D → Graphics
Ever confirmed: true
Flags: needinfo?(bas)
Keywords: regression,
testcase
Whiteboard: [MemShrink]
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Loic from comment #2) > Regression range: > https://hg.mozilla.org/integration/mozilla-inbound/ > pushloghtml?fromchange=b4c27a > b3cafeef188e84a704cbe72f27419a1eea&tochange=e4944eb37eecb1587c5acec96a14bd7f7 > d6f > de57 > > Bas Schouten — Bug 1221616: Use ID2D1CommandList instead of a bitmap for > temporary D2D drawing. r=jrmuizel > > Bas, could you check why your patch is leaking memory when drawing on canvas? It's not actually leaking, it's just.. leaving it around longer than it should, when you delete the canvas it will go away. Can you verify this issue isn't occurring on trunk? I believe this should be fixed on all branches.
Flags: needinfo?(bas)
The default behaviour is bad, memory use grows up indefinitely and disappears only if the user closes the tab. I tried with Nightly and in 1-2 min, the memory of the plugin-container (e10s was on) grew up to 500 MB before closing the tab.
OK, let's sort out if this is a problem in Nightly and any other branch. If this is going to increase the number of OOMs, we probably can't afford it.
Assignee: nobody → bas
Comment 6•8 years ago
|
||
It would also be useful to know if this happens with/without e10s enabled.
Assignee | ||
Comment 7•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/43581/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/43581/
Attachment #8736893 -
Flags: review?(jmuizelaar)
Updated•8 years ago
|
Attachment #8736893 -
Flags: review?(jmuizelaar) → review+
Comment 8•8 years ago
|
||
Comment on attachment 8736893 [details] MozReview Request: Bug 1259785: Do a proper flush when taking a snapshot so our dependent targets and command lists get appropriately cleared. r=jrmuizel https://reviewboard.mozilla.org/r/43581/#review40143
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/db49ab7e9e4f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Reporter | ||
Comment 11•8 years ago
|
||
It would be nice to have this fix released at least in FF47. Each day, many of our users are getting OOM errors. It'll take 3 months until FF48 is out, we'll have to force them to switch to another browser meanwhile.
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to Daniel Szabo from comment #11) > It would be nice to have this fix released at least in FF47. Each day, many > of our users are getting OOM errors. It'll take 3 months until FF48 is out, > we'll have to force them to switch to another browser meanwhile. It's -incredibly- unlikely that this problem is responsible for any of there issues, it requires a very specific set of circumstances. But yes, this patch is perfectly safe to uplift.
Flags: needinfo?(bas)
Comment 14•8 years ago
|
||
Comment on attachment 8736893 [details] MozReview Request: Bug 1259785: Do a proper flush when taking a snapshot so our dependent targets and command lists get appropriately cleared. r=jrmuizel Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: Canvas is using more memory on Windows with D2D. Nothing critical, but the patch is simple and safe to uplift. [Describe test coverage new/current, TreeHerder]: [Risks and why]: Low rist, simple patch. [String/UUID change made/needed]: None. I'm somewhat hesitant to uplift all the way to Beta since it's not critical, but since this is a simple patch we could have it at least on aurora.
Attachment #8736893 -
Flags: approval-mozilla-beta?
Attachment #8736893 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 15•8 years ago
|
||
(In reply to Bas Schouten (:bas.schouten) from comment #13) > (In reply to Daniel Szabo from comment #11) > > It would be nice to have this fix released at least in FF47. Each day, many > > of our users are getting OOM errors. It'll take 3 months until FF48 is out, > > we'll have to force them to switch to another browser meanwhile. > > It's -incredibly- unlikely that this problem is responsible for any of there > issues, it requires a very specific set of circumstances. But yes, this > patch is perfectly safe to uplift. The testcase was isolated from an online game. Users having OOMs all have FF45, no OOM on Chrome/IE or pre FF45. On my PC, using FF46b5 memory footprint is growing at a rate of 100MB/25secs while running the testcase.
Comment 16•8 years ago
|
||
(In reply to Daniel Szabo from comment #15) > The testcase was isolated from an online game. Users having OOMs all have > FF45, no OOM on Chrome/IE or pre FF45. > On my PC, using FF46b5 memory footprint is growing at a rate of 100MB/25secs > while running the testcase. This might be something else that needs to be fixed, then. Can you run this test case on the latest nightly and let us know if the problem persists?
Flags: needinfo?(szdy12)
Reporter | ||
Comment 17•8 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #16) > (In reply to Daniel Szabo from comment #15) > > The testcase was isolated from an online game. Users having OOMs all have > > FF45, no OOM on Chrome/IE or pre FF45. > > On my PC, using FF46b5 memory footprint is growing at a rate of 100MB/25secs > > while running the testcase. > > This might be something else that needs to be fixed, then. Can you run this > test case on the latest nightly and let us know if the problem persists? Latest nightly works fine, memory usage stays constant.
Flags: needinfo?(szdy12)
Comment 18•8 years ago
|
||
Comment on attachment 8736893 [details] MozReview Request: Bug 1259785: Do a proper flush when taking a snapshot so our dependent targets and command lists get appropriately cleared. r=jrmuizel May help with OOM issues, ok to uplift to aurora. We are heading into late beta so it seems best we keep this to ship with 47.
Attachment #8736893 -
Flags: approval-mozilla-beta?
Attachment #8736893 -
Flags: approval-mozilla-beta-
Attachment #8736893 -
Flags: approval-mozilla-aurora?
Attachment #8736893 -
Flags: approval-mozilla-aurora+
Comment 19•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/6b279bfa8288
Updated•8 years ago
|
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•