1427033 - Throttle animations in 0-opacity element subtree

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect)

Description

[bintoro]

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.

Comment 1

[bintoro]

> (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.

Comment 2

[Daosheng Mu[:daoshengmu]]

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.

Comment 3

[Daosheng Mu[:daoshengmu]]

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.

Comment 4

[Matt Woodrow (:mattwoodrow)]

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.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

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.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

OK, the restyle is caused by dom::WindowBinding::get_pageYOffset, I guess the site calls window.pageYOffset in requestAnimationFrame callback repeatedly?  An evangelism issue?

Comment 7

[Matt Woodrow (:mattwoodrow)]

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.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: 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.

Comment 9

[Matt Woodrow (:mattwoodrow)]

(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.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(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.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Trying to make the bug tile clearer.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1427033 - Throttle animations in opacity: 0 subtrees. r=hiro

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

Comment 14

[Brian Birtles (:birtles, Feb 26~ parental leave)]

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

Comment 15

[Emilio Cobos Álvarez (:emilio)]

(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?

Comment 16

[Hiroyuki Ikezoe (:hiro)]

No, looks pretty reasonable!

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1427033 - Tests for animation throttling in opacity: 0 subtrees. r=hiro

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d1cef863b05
Throttle animations in opacity: 0 subtrees. r=hiro

Comment 19

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/2d1cef863b05

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7e59b27d48d
Tests for animation throttling in opacity: 0 subtrees. r=hiro

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Ouch, the tests werent in the phab stack.

Comment 22

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/f7e59b27d48d