1574538 - Enable CoreAnimation by default (gfx.core-animation.enabled)

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, task, P3)

Description

[Markus Stange [:mstange]]

Bug 1491442 added support for CoreAnimation-based compositing. It's currently off by default (behind the pref gfx.core-animation.enabled).

This bug tracks enabling it by default.

Here's a green try push with the pref flipped: https://treeherder.mozilla.org/#/jobs?repo=try&revision=269fd0f81f44a6a97980cb89947c3d2edf116b9e

Comment 1

[Markus Stange [:mstange]]

I'm going to wait for bug 1491448 before making this change so that we don't regress full screen video. But once that's landed, flipping the pref should be a slight win in terms of power usage and compositing efficiency. But the real wins will come from the other dependencies of bug 1429522.

Comment 2

[Markus Stange [:mstange]]

Here's a Talos comparison between the pref off and on; click "Show only important changes" to get something that's easier to read.
It shows a number of wins across the board and one regression.
The regression is probably not real: The glvideo test is already very bimodal and I think the "pref on" jobs probably just happened to hit the "high" values more frequently.

The improvements are partially real and partially a change in what's being tested.
BasicCompositor is a lot more efficient with the CoreAnimation path because one or two window-sized copies are eliminated, so the 20% win on basic_compositor_video is probably real.
Window resizing also becomes a lot more efficient with CoreAnimation: Without CoreAnimation, the window has a main memory buffer behind the OpenGL context which is never shown on the screen but which gets copied around a bit during window resizing. There's also some extra surface synchronization overhead. CoreAnimation gets rid of that copy and overhead. So the 17% win on tresize is probably also real.
The other wins are not real; they're caused by a difference in how ASAP mode works.

"ASAP" mode is enabled by setting layout.frame-rate to 0. This causes us to refresh the window as many times as we can, more frequently than vsync if possible, and to turn off refresh synchronization on the OpenGL context.
With the non-CoreAnimation path, this has the effect that our window contents are presented to the screen as soon as possible, potentially incurring "tearing" artifacts. My hypothesis for how this works is that every call to SwapBuffers forces the window server to do a present in the window's rectangle and copy the window contents to the screen's front buffer, and I think that SwapBuffers waits for the previous frame's window server copy to finish. So our measurement included measuring one window server copy per frame.
With CoreAnimation, we no longer have a way to force an "immediate window server copy". We draw as frequently as we can, but the window server will only copy our window contents to the screen once per vsync interval. So the ASAP mode measurements now include less window server work, maybe one window server copy every 10 drawn frames or so, depending on how many composites we do within one vsync interval.

I'm not concerned about this change in measurement. We will still measure our own drawing work just as we were before. It would be nice to get a good sense of how much window server work each composite is causing, but it's not the end of the world if we can't get that data.

Comment 3

[Asif Youssuff]

Does the new preference work with WebRender or is it better to use standard compositing while testing the new preference? I have been using WebRender on a Macbook Air with minor issues and would prefer to use it with CoreAnimation if possible.

Comment 4

[Markus Stange [:mstange]]

The pref works with WebRender but not all upcoming optimizations apply to WebRender. Bug 1491448 will help WebRender but the other planned short-term optimizations will not affect WebRender.
Once bug 1491448 lands, it is recommended to flip the pref on when testing WebRender. But I'll flip the pref by default at that point anyway.

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1574538 - Enable CoreAnimation by default. r=jrmuizel

Comment 6

[Markus Stange [:mstange]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3377456c4b56a586d622a35211af4a0dcc5dd030

Unfortunately there's one real test failure in that try push:

sync IPC PCompositorBridge::Msg_FlushRendering 1 more times than expected

I'll need to look into it before this can land. We don't want to have two synchronous composites when opening windows; one is enough.

Comment 7

[Markus Stange [:mstange]]

Filed bug 1575419 for the test failure.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eeeb0858e706
Enable CoreAnimation by default. r=jrmuizel

Comment 9

[Dorel Luca [:dluca]]

Backed out changeset eeeb0858e706 (bug 1574538) for mochitest failures on toolkit/content/tests/widgets/test_videocontrols.html

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=262616683&repo=autoland&lineNumber=38575

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=eeeb0858e706513fa2a3b9e23b5487322cd22f24

Backout:
https://hg.mozilla.org/integration/autoland/rev/eaa150b76520436278334337b869f22634faccdc

Comment 10

[Markus Stange [:mstange]]

I've put up a fix in bug 1514060 (and another one in bug 1575630). The reason this test started failing permanently with CoreAnimation is that CoreAnimation makes window resizing paints asynchronous, see bug 1575630 for more information.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/152bfc05a509
Enable CoreAnimation by default. r=jrmuizel

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/152bfc05a509

Comment 13

[Albert Scheiner [:alberts]]

👏

Comment 14

[Flore Allemandou [:flore]]

Hi it seems that this is the cause of bug 1576399

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you suggest a release note (for beta 70)? Is this shipping in 70 preffed on or do you plan a gradual rollout?

Comment 16

[Markus Stange [:mstange]]

I'm planning to write a release note, but the contents of the release note will depend on what other changes end up making it into 70. What date is the release note needed by?

Comment 17

[Markus Stange [:mstange]]

Oh, and yes, this is shipping in 70 unless we discover a deal breaker. No gradual rollout.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

For beta, any day is fine this week. We can modify it as needed during beta or when you know what we will ship in release.

Comment 19

[Markus Stange [:mstange]]

How about: "macOS: Vastly reduced power consumption with a more efficient compositor."

I considered adding a "during most browsing work loads" qualifier but I think it's unnecessary. Really, anything except full screen video should be benefiting. And while I still haven't done any scientific tests to gather reproducible numbers, the user reports in bug 1429522 are convincing enough that the word "vastly" should be justified, even before the other power reduction patches land (and even if those don't make it into 70).

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

That's fantastic, thank you! There are a few bugs open about mac power issues and I'll be poking around to see if we can mark them fixed and verified!

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

I'd love to be able to link to a blog post on this fix (not for 70 beta necessarily but maybe for the 70 release timeframe)

Comment 22

[Nomis101]

Its in the news already: https://appleinsider.com/articles/19/09/03/firefox-macos-test-build-is-far-better-for-your-battery-life