Closed
Bug 1454824
Opened 6 years ago
Closed 6 years ago
Disable discarding of animated image frames
Categories
(Core :: Graphics: ImageLib, defect, P3)
Tracking
()
VERIFIED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | verified |
firefox59 | --- | unaffected |
firefox60 | + | verified |
firefox61 | + | verified |
firefox62 | --- | verified |
People
(Reporter: aosmond, Assigned: aosmond)
References
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(1 file)
767 bytes,
patch
|
tnikkel
:
review+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
There are a number of bugs open (bug 1444537 and bug 1444537) due to the landing of bug 523950. These problems are not insurmountable, but we are getting too close to release to want to land anything risky. Hence I propose we increase the image.animated.decode-on-demand.threshold-kb threshold from 20MB to ~4GB to avoid the algorithm trigger causing the discarding of frames. Since most animated images have a smaller footprint than 20MB, the path taken by the larger images is well tested. It will be very similar behaviour as to that in prior releases as well.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → aosmond
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•6 years ago
|
||
I selected 4194303 (or (2^32 / 1024) - 1) because 4194303 * 1024 won't overflow a 32-bit size_t in the calculations in AnimationSurfaceProvider::AnimationSurfaceProvider. https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/image/AnimationSurfaceProvider.cpp#44 try (64-bit): https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7f3d8e092e72a5848169fd9c056ff98e8ca5f07 try (32-bit): https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f8f16b0bd816798590e7370ee2b83a554bd8756
Attachment #8968709 -
Flags: review?(tnikkel)
Updated•6 years ago
|
Attachment #8968709 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 2•6 years ago
|
||
Comment on attachment 8968709 [details] [diff] [review] bug1454824.patch, v1 Approval Request Comment [Feature/Bug causing the regression]: Bug 523950 [User impact if declined]: High CPU use as described in bug 1444537 and bug 1454149. May experience shutdown hangs as well. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: No, the pref change is only for beta. [Needs manual test from QE? If yes, steps to reproduce]: Go to the github.com page from bug 1454149 and verify the CPU usage is similar or better as compared to 59. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: It is a simple pref change increasing the threshold where we decide to start discarding animated image frames to something very high such that it will never trigger. The code path where we don't discard frames is very well tested as that is most animated images already. The behaviour will be a reversion to what was shipped in 59 and in previous releases. [String changes made/needed]: None.
Attachment #8968709 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•6 years ago
|
status-firefox59:
--- → unaffected
status-firefox60:
--- → affected
Priority: -- → P3
Whiteboard: [gfx-noted]
Target Milestone: --- → mozilla60
Version: unspecified → 60 Branch
Assignee | ||
Updated•6 years ago
|
Comment 3•6 years ago
|
||
Comment on attachment 8968709 [details] [diff] [review] bug1454824.patch, v1 pref change to avoid triggering a regression with animated gifs, approved for 60.0b14
Attachment #8968709 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 4•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/893ccda7531f
Updated•6 years ago
|
Flags: qe-verify+
Updated•6 years ago
|
Keywords: regression
Updated•6 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment 5•6 years ago
|
||
I have managed to reproduce this issue on Firefox 61.0a1 (BuildId:20180417225505) while using the following webpage: -https://blog.github.com/2016-08-19-building-your-first-atom-plugin/ from Bug 1454149 (100% CPU usage). This issue is verified fixed as the CPU usage is much lighter on Firefox 60.0b16 (BuildId:20180426170554) using Windows 10 64bit, macOS 10.13 and Ubuntu 16.04 32bit.
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a22564076cae Disable discarding of animated image frames due to high CPU consumption. r=tnikkel
Assignee | ||
Comment 7•6 years ago
|
||
I decided to land this on 61 as well, since I think we aren't going to make the window there either...
Summary: Disable discarding of animated image frames for 60 release → Disable discarding of animated image frames
Comment 8•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a22564076cae
Comment 9•6 years ago
|
||
Let's get a verification run on 61 as well.
Status: VERIFIED → RESOLVED
Closed: 6 years ago → 6 years ago
status-firefox61:
--- → fixed
tracking-firefox60:
--- → +
tracking-firefox61:
--- → +
Flags: qe-verify+
Target Milestone: mozilla60 → mozilla61
Updated•6 years ago
|
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → fixed
Comment 10•6 years ago
|
||
This issue is verified fixed on Firefox 61.0b3 (BuildId:20180507191226), Firefox 62.0a1 (BuildId:20180507222648) and Firefox 60.0esr (BuildId:20180503164101) using Windows 10 64bit, macOS 10.13.3 and Ubuntu 16.04 64bit.
You need to log in
before you can comment on or make changes to this bug.
Description
•