Closed Bug 1427033 Opened 5 years ago Closed 2 years ago

Throttle animations in 0-opacity element subtree

Categories

(Core :: DOM: Animation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla75
Performance P2
Tracking Status
firefox75 --- fixed

People

(Reporter: kalle, Assigned: emilio)

References

Details

(Keywords: perf:resource-use, power, Whiteboard: [gfx-noted] [layout:triage-discuss])

Attachments

(3 files)

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:59.0) Gecko/20100101 Firefox/59.0
Build 20171224140229
macOS 10.13.1


I thought I'd document a couple of fresh observations of abnormal CPU consumption on seemingly innocuous pages. Can anyone reproduce these? Is this a Mac-specific issue? I've seen reports of high CPU usage on Macs post-Quantum.


(1) Viewing any product page on Zalando.com causes extreme CPU usage:

- main process ~70%
- content process ~30%
- WindowServer ~30%

On Safari, opening a product page makes WindowServer consume a bit of CPU (<10%), while the main and content processes remain at less than 1%.


(2) Viewing any article on Engadget.com causes roughly 60% constant CPU usage, most of this in the content process.

Example URL: https://www.engadget.com/2017/12/24/qualcomm-self-driving-tech-california/

Again, Safari consumes only a negligible amount of CPU on the same page.
> (1) Viewing any product page on Zalando.com causes extreme CPU usage:
> 
> - main process ~70%
> - content process ~30%
> - WindowServer ~30%

More info: the CPU burden goes away completely after scrolling down so that the main product image is outside the viewport.
Status: UNCONFIRMED → NEW
Component: General → Canvas: 2D
Ever confirmed: true
Keywords: power
OS: Mac OS X → All
Product: Firefox → Core
Hardware: x86_64 → All
The CPU usage is about 70% when entering the web page (https://www.engadget.com/2017/12/24/qualcomm-self-driving-tech-california/
) even making the page to be displayed for a while on my MBP.

However, I don't think it is a Canvas2D issue, it would be possible to be a layout or compositing problem.
I run it with http://perf-html.io, and it tells us nsLayoutUtils::PaintFrame() takes about 94% time. It should be a layout issue , and it can be reproduced in FF 57 and FF 59.
Component: Canvas: 2D → Layout: Web Painting
Whiteboard: [gfx-noted]
Looks like this page has a couple of 'spinner' animations (over the 'Add to wish list' and 'add to bag' buttons), hidden by an ancestor with opacity:0.

The animations are ticking at 60fps, so we're running a full paint cycle each time only to figure out that nothing visible has actually changed.

Hiro, do we have any plans to try avoid this work?

The animations aren't running async, which I guess may be because they don't actually get a Layer to be attached to?

On the site end, they could fix this by using display:none instead of opacity:0.
Flags: needinfo?(hikezoe)
Priority: -- → P2
Hmm, those animations (on class='sprc-T0Wq') I can see are being throttled by offscreen throttling mechanism (an ancestor has 'height: 0 !important' style). 

I see requestAnimationFrame callbacks in devtools so there might be something that triggers restyles other than animations.  Keep NI.
OK, the restyle is caused by dom::WindowBinding::get_pageYOffset, I guess the site calls window.pageYOffset in requestAnimationFrame callback repeatedly?  An evangelism issue?
Flags: needinfo?(hikezoe)
Hiro, sorry, I wasn't very clear as to which page I was looking at!

The URL I looked at is: https://www.zalando.co.uk/adidas-performance-adissage-sandals-ad542b1iy-q11.html

The style with an animation is ".h-action.-button .h-action-states.-loading .h-action-icon::before"

Removing the two elements that create the ::before pseudos with the animation gets rid of the CPU usage/rAF firing.
Flags: needinfo?(hikezoe)
Attached file A reduced test case
Oh I see.  Now I can see the animation in question. :) I haven't had such plan but it's definitely worth doing.
Flags: needinfo?(hikezoe)
Component: Layout: Web Painting → DOM: Animation
See Also: → 1237454
(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> OK, the restyle is caused by dom::WindowBinding::get_pageYOffset, I guess
> the site calls window.pageYOffset in requestAnimationFrame callback
> repeatedly?  An evangelism issue?

Ok, so my understanding here is that we try to throttle the transform animation, but the site keeps trying to read style data which triggers a flush (and breaks throttling).

We could look into not issuing a SchedulePaint/InvalidateFrame call if the style update happened for a throttled animation.

I'm also working on patches for retained-dl which would detect that nothing in the display list has changed, and would skip FrameLayerBuilder/compositing etc.
(In reply to Matt Woodrow (:mattwoodrow) from comment #9)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> > OK, the restyle is caused by dom::WindowBinding::get_pageYOffset, I guess
> > the site calls window.pageYOffset in requestAnimationFrame callback
> > repeatedly?  An evangelism issue?
> 
> Ok, so my understanding here is that we try to throttle the transform
> animation, but the site keeps trying to read style data which triggers a
> flush (and breaks throttling).

Yes, that's right.

> We could look into not issuing a SchedulePaint/InvalidateFrame call if the
> style update happened for a throttled animation.

Yeah, but I suspect we can't optimize in this specific case where the flush caused by window.pageYOffset since, IIUC,  transform animations might affect scroll position.
Trying to make the bug tile clearer.
Summary: Some websites cause excessive CPU consumption → Throttle animations in 0-opacity element subtree
Blocks: 1530071
Whiteboard: [gfx-noted] → [gfx-noted] [qf]

P2 was set 2 years ago. Maybe this should go through re-triage again? Seeing such high CPU consumption across different web sites is disappointing and frustrating, especially when being not connected to AC.

Priority: P2 → --
Component: DOM: Animation → CSS Transitions and Animations
Whiteboard: [gfx-noted] [qf] → [gfx-noted] [qf] [layout:triage-discuss]
Whiteboard: [gfx-noted] [qf] [layout:triage-discuss] → [gfx-noted] [qf:p2:resource] [layout:triage-discuss]
Depends on: 1618303

I hope you can help me figure out writing tests for this and figuring whether
the reasoning here is correct / I'm missing something else :)

Depends on D64441

Assignee: nobody → emilio
Status: NEW → ASSIGNED

Pretty sure this should still be DOM: Animation (since it's about the underlying animation primitives, not the specific mapping to transitions/animations properties).

(In reply to Brian Birtles (:birtles) from comment #14)

Pretty sure this should still be DOM: Animation (since it's about the underlying animation primitives, not the specific mapping to transitions/animations properties).

Sure, fair enough.

Ni? myself for writing tests tomorrow of:

  • Basic throttling.
  • (non-)throttling when root of the opacity zero subtree is animating (basically, the nasty test-case from hiro's review comment :P).
  • throttling after an animation towards zero from the root.
  • Throttling of out-of-flow descendants (as opacity does apply to them).

Anything else that may be missing hiro?

Component: CSS Transitions and Animations → DOM: Animation
Flags: needinfo?(hikezoe.birchill)
Flags: needinfo?(emilio)

No, looks pretty reasonable!

Flags: needinfo?(hikezoe.birchill)
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d1cef863b05
Throttle animations in opacity: 0 subtrees. r=hiro
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f7e59b27d48d
Tests for animation throttling in opacity: 0 subtrees. r=hiro

Ouch, the tests werent in the phab stack.

Regressions: 1630919
Blocks: 1589294
Performance: --- → P2
Whiteboard: [gfx-noted] [qf:p2:resource] [layout:triage-discuss] → [gfx-noted] [layout:triage-discuss]
You need to log in before you can comment on or make changes to this bug.