Closed
Bug 1454149
Opened 7 years ago
Closed 7 years ago
https://blog.github.com/2016-08-19-building-your-first-atom-plugin/ uses 100% CPU due to GIF
Categories
(Core :: Graphics: ImageLib, defect, P3)
Core
Graphics: ImageLib
Tracking
()
VERIFIED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | + | fixed |
firefox62 | + | fixed |
firefox65 | --- | verified |
firefox66 | --- | verified |
firefox67 | --- | verified |
People
(Reporter: mayankleoboy1, Assigned: aosmond)
References
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
go to https://blog.github.com/2016-08-19-building-your-first-atom-plugin/
Let the GIF load
Watch the CPU usage
uses 75-100% of my CPU
https://perfht.ml/2ELbbFd
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(aosmond)
Assignee | ||
Comment 1•7 years ago
|
||
Regression from bug 523950. Unfortunately unrelated to bug 1444537 from what I can tell (it would be nice to have an STR).
CPU usage is high even in release, but at least drops off as the decoding completes, at the cost of using a few GB of RAM. However comparing to Chrome, I see that it doesn't even advance animated images which are not in view. I think that should be the solution.
Depends on: 523950
Assignee | ||
Updated•7 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(aosmond)
Priority: -- → P3
Whiteboard: [gfx-noted]
Assignee | ||
Comment 2•7 years ago
|
||
I think this is the simplest change I can make which should fix the problem, in addition to making this more efficient in general for animated images outside the current view.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1d87311384c4c433aac6f8427fdc9437cd045bf
This patch will make us ignore refresh ticks for an animated image if nothing is accessing its composited frame. This should work for all frame accessing paths for a RasterImage -- Draw, GetFrame, GetFrameAtSize, GetImageContainer, UpdateImageContainer. The image container case will still have a high CPU usage if the entity which requested the image container keeps it around, but doesn't otherwise use it (bad entity!); since animated images shouldn't be layerized, I don't think this is a big concern (correct me if I'm wrong -- we still need to solve this for WebRender as well!).
I think it should be upliftable to beta, assuming the try is green, but I'll let my reviewer make that call. If it is too risky, we need to increase image.animated.decode-on-demand.threshold-kb to the highest value for beta to delay the feature to the next release.
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 3•7 years ago
|
||
Comment on attachment 8968293 [details] [diff] [review]
0001-Bug-1454149-Do-not-advance-animated-images-which-are.patch, v1
Disable review for now. We discussed this on IRC, and it definitely needs more work.
Attachment #8968293 -
Flags: review?(tnikkel)
Assignee | ||
Comment 4•7 years ago
|
||
This should take care of discarded images, as well as adding a pref to disable this feature if it causes problems after making its way into a build.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6703abeb8756257005433e07b3d421a0e2ed2e7
I still need to dig into the bug history to find the reasons why we hesitated to change this behaviour in the past.
Attachment #8968293 -
Attachment is obsolete: true
Assignee | ||
Comment 5•7 years ago
|
||
I can't find any bugs discussing the "we should advance animated images when out of view" issue. Maybe you can dig it up :).
Attachment #8970294 -
Attachment is obsolete: true
Attachment #8970660 -
Flags: review?(tnikkel)
Updated•7 years ago
|
Keywords: regression
Updated•7 years ago
|
Blocks: 523950
status-firefox59:
--- → unaffected
status-firefox60:
--- → affected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → affected
tracking-firefox61:
--- → +
No longer depends on: 523950
Assignee | ||
Comment 6•7 years ago
|
||
The landing of bug 1454824 should have mitigated the problem on 60, so I'm marking it as unaffected. We'll still want a patch for 61 though.
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Marion Daly [:mdaly] from comment #7)
> Where are we on review?
I decided to disable the regressing feature, just landed the pref change so it should make it into 61 beta as normal. Once it is merged I'll mark 61 as unaffected here.
Flags: needinfo?(aosmond)
Updated•7 years ago
|
Attachment #8970660 -
Flags: review?(tnikkel) → review+
Comment 9•7 years ago
|
||
(In reply to Andrew Osmond [:aosmond] from comment #5)
> I can't find any bugs discussing the "we should advance animated images when
> out of view" issue. Maybe you can dig it up :).
I did a quick look, didn't find anything.
Comment 10•7 years ago
|
||
This is technically "fixed" for all affected versions now since the disabling landed on m-c directly, but I'm assuming we want to leave this bug open for fixing the underlying problem (and re-enabling it for 62?).
In the mean time, requesting QA to at least give this a spot check on 61 betas to ensure that things are looking good there again.
Comment 11•7 years ago
|
||
Pushed by aosmond@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba377bd503e1
Do not advance animated images which are not displayed. r=tnikkel
Comment 12•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Comment 13•7 years ago
|
||
bugherder |
Comment 14•7 years ago
|
||
The issue is still reproducible for me on 61.0b3 and 62.0a1 (2018-05-09). From what I observed, the cpu usage is around 60% utilization when scrolling the page, and firefox's ram usage is jumping all around (as example one second is at 1.5gb usage, then jumps to 700mb, then back up to 2.0gb). If I stop scrolling, the usage stays still at around 700mb, but as soon as the scrolling starts, the memory jumps back to a higher value. (Windows 10 system - Amd FX8320, 16gb DDR3, Radeon Integrated HD3000).
On macOS 10.12 and 10.13, the cpu usage stays at 130-140%, but the memory usage is not touched.
Assignee | ||
Comment 15•7 years ago
|
||
(In reply to Sasca Catalin, QA [:csasca] from comment #14)
> The issue is still reproducible for me on 61.0b3 and 62.0a1 (2018-05-09).
> From what I observed, the cpu usage is around 60% utilization when scrolling
> the page, and firefox's ram usage is jumping all around (as example one
> second is at 1.5gb usage, then jumps to 700mb, then back up to 2.0gb). If I
> stop scrolling, the usage stays still at around 700mb, but as soon as the
> scrolling starts, the memory jumps back to a higher value. (Windows 10
> system - Amd FX8320, 16gb DDR3, Radeon Integrated HD3000).
> On macOS 10.12 and 10.13, the cpu usage stays at 130-140%, but the memory
> usage is not touched.
Is the beta CPU usage high compared to 59? This was never a great page for us. Chrome should be similar as long as an animation is not in view. I expect memory usage to be very high on release/beta.
Today's nightly should similarly to Chrome. The memory footprint should be lower than beta, and the CPU footprint somewhat higher (there is a tradeoff for the lower memory usage). Unfortunately we aren't quite as good as Chrome with respect to out of view animated images, but if you stop scrolling around "Activation commands", the CPU should also hit zero. This is because we appear to be prerendering more parts above and below the current view, and it captures the animated images more frequently.
To confirm, the expected preferences on nightly are:
image.animated.decode-on-demand.threshold-kb == 20480
image.animated.resume-from-last-displayed == true
Comment 16•7 years ago
|
||
Hi Andrew. On 59.0.3 and 61.0b3, the cpu gets to 50%, then it slowly stabilizes at around 20%. On 62.0a1 (2018-05-09) it stays at around 35-40% cpu usage. The ram is jumping all around on every firefox build and if the page is scrolled all the way down, firefox has a ram usage of around 4gb and is jumping to 2gb then to 3gb, again to 4gb and so on. Chrome tops it's cpu usage to 25-30% and the memory usage is nearly untouched (200-400mb).
Assignee | ||
Comment 17•7 years ago
|
||
(In reply to Sasca Catalin, QA [:csasca] from comment #16)
> Hi Andrew. On 59.0.3 and 61.0b3, the cpu gets to 50%, then it slowly
> stabilizes at around 20%. On 62.0a1 (2018-05-09) it stays at around 35-40%
> cpu usage. The ram is jumping all around on every firefox build and if the
> page is scrolled all the way down, firefox has a ram usage of around 4gb and
> is jumping to 2gb then to 3gb, again to 4gb and so on. Chrome tops it's cpu
> usage to 25-30% and the memory usage is nearly untouched (200-400mb).
I think bug 1382683 is going to help with the CPU usage. The memory is surprisingly, I'll need to think further on that.
Comment 18•6 years ago
|
||
Hi Andrew. Have you managed to find something about memory usage?
Flags: needinfo?(aosmond)
Assignee | ||
Comment 19•6 years ago
|
||
I think that recycling might help with the memory usage. For non-WebRender users, everything you need is already in tree (nightly only). All you need to do is set image.animated.generate-full-frames to true, restart the browser, and check the memory usage. Since we are reusing the buffers now, the footprint shouldn't be constantly jumping around and thus might look better in the snapshots.
Flags: needinfo?(aosmond)
Comment 20•6 years ago
|
||
Verified as fixed on Firefox Nightly 67.0a1 (2019-02-07) on Windows 10(64-bit)
**On a side note the memory usage still fluctuates anywhere from 1 GB to 3 GB
Status: RESOLVED → VERIFIED
status-firefox67:
--- → verified
Comment 21•6 years ago
|
||
I've also verified it on Firefox 65.0(2019-01-24) and Firefox 66.0b5(2019-02-04)
You need to log in
before you can comment on or make changes to this bug.
Description
•