1771902 - Compositor CSS animations are not paused in fully occluded windows

Status: NEW

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Florian Quèze [:florian]]

Here is a profile: https://share.firefox.dev/3lZyBPe

The 'Renderer' thread is using a lot of CPU in the parent process. Looking at the 'Composite' markers there (and the "Render reason ANIMATED_PROPERTY" markers), and the 'CompositorScreenshot' markers in the main thread, I reached the conclusion that we are animating the cursor in 2 google documents. This is confirmed by looking at the CSS Animation markers in the child process with the pid 19661, we are animating the opacity property on the compositor for the node div@28d552d80 class="kix-cursor docs-ui-unprintable docs-text-ui-cursor-blink".

Looking at the 'SampleAnimation' markers in the parent process Compositor thread, it's clear that both the 19661 and 19668 content processes are running opacity animations on the compositor, but only 19661 has RefreshDriverTick and CSS animation iteration markers.

19661 has the process priority "foreground"; 19668 has the priority "background". Both windows are fully occluded. I'm not sure why 19661 keeps having 120Hz refresh driver ticks for a window that's occluded, but the "foreground" priority is likely due to the same content process having a pinned tab. If somehow the window didn't get marked as occluded, it's possible that it happened as a result of me unplugging an external screen (ie. maybe it wasn't occluded on the external screen, and became fully occluded when I unplugged and it was moved to the primary screen of the macbook).

The behavior in the content process 19668 is probably already covered by bug 1768495.

The behavior in the content process 19661 is still unclear to me, so help to understand what happened would be welcome.

Other things I noticed in the profile:

• (looking at the flame graph of the main thread of content process 19661) We are in a "SpinEventLoopUntil prompts/Prompter.jsm:openPromptSync" nested event loop. Does this play a role?
• there's a lot of IPC traffic to send PVsync::Msg_Notify messages from the parent to the content process 19661. In the marker chart, we have "RefreshDriverTick - Tick reasons: HasObservers (DocumentTimeline animations [Style])" markers but nothing else seems to be happening. There are "Perform microtasks" markers, but no "requestAnimationFrame callback" markers, so I don't think any JS of the page is being executed. The only DOM events being dispatched at "animationiteration" events, I don't think they require 120Hz notifications. Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?
• Looking at the Renderer and Compositor threads in the parent, there are Composite markers even when sampling the animation didn't result in any change: https://share.firefox.dev/3x15OyD Is there room for more optimization there? Zooming in to look at the order of markers (https://share.firefox.dev/3NJ9Nqn), we have first the "CompositeToTarget" markers, then the "SampleAnimation" markers, then the "Composite" markers.

Markus, do you have answers to the 2 questions above and/or other observations based on the profile? (feel free to forward the needinfo)

Comment 1

[Markus Stange [:mstange]]

(In reply to Florian Quèze [:florian] from comment #0)

We are in a "SpinEventLoopUntil prompts/Prompter.jsm:openPromptSync" nested event loop. Does this play a role?

I don't know. It shouldn't, but it might.

The only DOM events being dispatched at "animationiteration" events, I don't think they require 120Hz notifications. Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?

I would like this too, but this is a question for Hiro.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

(In reply to Markus Stange [:mstange] from comment #1)

The only DOM events being dispatched at "animationiteration" events, I don't think they require 120Hz notifications. Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?

I would like this too, but this is a question for Hiro.

IIRC, there's an open bug for this case, I can't find it now though.

If our refresh driver had a way to ask invoking a given function in a given time, something like "hey refresh driver, please invoke this function 10s later" or some such, we could optimize it. It will require rewriting whole animation event handling stuff though. Also I am not sure whether it will give us a performance win in most cases, say if there's a lot for short duration animations, probably it will be worse than the current implementation I guess.

Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?

Note that even if the animation in question is running on the compositor, discarding the animation is triggered on the main-thread. We used to discard the compositor animations based on the time stamp on the compositor thread, but it caused graphical glitches so that now we discard them based on the time stamp on the main-thread.

Comment 3

[Florian Quèze [:florian]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)

(In reply to Markus Stange [:mstange] from comment #1)

The only DOM events being dispatched at "animationiteration" events, I don't think they require 120Hz notifications. Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?

I would like this too, but this is a question for Hiro.

IIRC, there's an open bug for this case, I can't find it now though.

If our refresh driver had a way to ask invoking a given function in a given time, something like "hey refresh driver, please invoke this function 10s later" or some such, we could optimize it.

I don't understand the need for invoking the function from the refresh driver. Couldn't the animationiteration event be generated from a timer running on the content process, rather than from refresh driver ticks, if we know the animation is running on the compositor?

Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?

Note that even if the animation in question is running on the compositor, discarding the animation is triggered on the main-thread.

And from reading this, it seems there's already some handling of the 'end of animation' happening on the content process main thread. So I guess I don't understand why the content process receives vsync notifications.

More generally, I'm not sure what the next step should be in this bug.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

(In reply to Florian Quèze [:florian] from comment #3)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)

(In reply to Markus Stange [:mstange] from comment #1)

The only DOM events being dispatched at "animationiteration" events, I don't think they require 120Hz notifications. Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?

I would like this too, but this is a question for Hiro.

IIRC, there's an open bug for this case, I can't find it now though.

If our refresh driver had a way to ask invoking a given function in a given time, something like "hey refresh driver, please invoke this function 10s later" or some such, we could optimize it.

I don't understand the need for invoking the function from the refresh driver. Couldn't the animationiteration event be generated from a timer running on the content process, rather than from refresh driver ticks, if we know the animation is running on the compositor?

We could, if the timer is as precise as refresh driver's vsync time stamp. Anyway, it's unrelated whether the animation is on the compositor or not.

Could we avoid notifying content processes of every vsync tick when the only thing that is happening is an animation that is fully running on the compositor?

Note that even if the animation in question is running on the compositor, discarding the animation is triggered on the main-thread.

And from reading this, it seems there's already some handling of the 'end of animation' happening on the content process main thread. So I guess I don't understand why the content process receives vsync notifications.

The content process vsync is the main source to tell when we fire animation events, when we stop animations, etc. etc. Without it we can't control animations reliably/precisely.

More generally, I'm not sure what the next step should be in this bug.

I am not sure either. Using timer-base event handling would work, but it's not easy, it will be a tough work and will be messy and honestly I don't want to do it.

I am wondering that given that the contents inside occluded browser window keeps ticking, does it mean we've decided not throttle refresh driver inside occluded window? why?

Comment 5

[Florian Quèze [:florian]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)

I am wondering that given that the contents inside occluded browser window keeps ticking, does it mean we've decided not throttle refresh driver inside occluded window? why?

Throttling the refresh driver for occluded windows has been implemented recently in bug 1578123.

Comment 6

[Bas Schouten (:bas.schouten)]

The Performance Priority Calculator has determined this bug's performance priority to be P1. If you'd like to request re-triage, you can reset the Performance flag to "?" or needinfo the triage sheriff.

Platforms: [x] Windows [x] macOS [x] Linux [x] Android
Websites affected: Major
[x] Causes severe resource usage

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, the Performance Impact field flags this bug as having a high impact on the performance.
:bhood, could you consider increasing the severity of this performance-impacting bug? Alternatively, if you think the performance impact is lower than previously assessed, could you request a re-triage from the performance team by setting the Performance Impact flag to ??

For more information, please visit auto_nag documentation.

Comment 8

[Bob Hood [:bhood]]

From what I can glean from the dependency chain here, this might have been addressed by https://bugzilla.mozilla.org/show_bug.cgi?id=1779559. Florian, can we get confirmation with a current release?

Comment 9

[Florian Quèze [:florian]]

(In reply to Bob Hood [:bhood] from comment #8)

Florian, can we get confirmation with a current release?

I can no longer reproduce on Google docs, but I'm not sure if that's related to the changes from bug 1779559 or caused by changes to the web page's JS code. When profiling, I see the page receives a "deactivate" even on the window object, and a div element receives a "blur" event. There's JS code running off of a listener for that blur event, that sets the "display" CSS property, and after that an "animationcancel" event.

I can reproduce using https://developer.mozilla.org/en-US/ (there's a big CSS animation on the MDN homepage, and it doesn't stop when the window is fully occluded).

Comment 10

[:Gijs (he/him)]

(In reply to Florian Quèze [:florian] from comment #9)

I can reproduce using https://developer.mozilla.org/en-US/ (there's a big CSS animation on the MDN homepage, and it doesn't stop when the window is fully occluded).

There's no longer a big CSS animation, AFAICT - do you have an alternative testcase?

Comment 11

[Florian Quèze [:florian]]

(In reply to :Gijs (he/him) from comment #10)

There's no longer a big CSS animation, AFAICT - do you have an alternative testcase?

https://fqueze.github.io/tests/test-background-anim.html

Comment 12

[Frank Doty [:fdoty]]

:bhood based on comment 8 and comment 11 it seems this issue has not been addressed. After speaking with :gijs and :bas, it is agreed that the High impact to performance currently stands. Can this bug be rated an S2? If not, can you provide rationale as to why it should remain S3 (or an other rating)? Thank you

Comment 13

[Bob Hood [:bhood]]

Adding to Graphics triage to evaluate.

Comment 14

[Bob Hood [:bhood]]

This actually seems more like a Layout issue. Hiro, any chance could you have a look here?

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Though this is more or less a gfx issue, indeed animation lies across both of layout and gfx.

Anyways, if CSS animations inside fully occluded windows still keeps consuming CPU, then it means we don't stop calling update_document, which is kinda unfortunate. If we stop calling the update_document function in such situations, the CSS animations also stop consuming CPU. (And it will also stop APZ animations as well)

If it's hard to do, we can stop CSS animations here with a check telling whether the window is fully occluded or not.

Sotaro, which approach do you think is more reasonable? If it's former, then it's out of scope of my knowledge. Even if it's latter, I don't know he way to tell the given window is fully occluded or not. So with either approach, I need your help. Thanks!

Comment 16

[Sotaro Ikeda [:sotaro]]

Hi :hiro, can you still reproduce the problem with latest Firefox?

nsIWidget::PauseOrResumeCompositor() pause compositor when window is fully occluded. It is added by Bug 1768495.

Comment 17

[Hiroyuki Ikezoe (:hiro)]

Yeah, it was unclear what fully occluded means so I tested. In the test case in comment 11, the animation on the compositor keeps running if it's in a browser window covered by another firefox window. FWIW I am on Wayland.

Comment 18

[Sotaro Ikeda [:sotaro]]

WindowOcclusion is supported only on Window and macOS. Then on Wayland, animation keeps running. When I tested on Windows and on macOS, I could not reproduce the problem.

Comment 19

[Hiroyuki Ikezoe (:hiro)]

Okay, then this bug has been already fixed? Florian?

Comment 20

[Jeff Fortin]

WindowOcclusion is supported only on Window and macOS. Then on Wayland, animation keeps running.

I have filed a similar issue in WebKitGTK recently.

I think this discussion with one of the Mutter developers might be helpful as reference for how to achieve this with Wayland:
https://gitlab.gnome.org/GNOME/mutter/-/issues/3634

Okay, then this bug has been already fixed?

If CPU usage with https://fqueze.github.io/tests/test-background-anim.html is any indication, then I am seeing this issue with Firefox 133.0.3 from Fedora 41's repositories, while running on GNOME 47's Wayland session; if I have another application's window (maximized or not) occluding—in part or entirely—Firefox's window with the testing link above, CPU usage remains constant even though the window is not actually visible in practice. It only falls to zero if I minimize the window, move it to another workspace, or switch tabs.