Closed Bug 1574538 Opened 3 months ago Closed 3 months ago

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

Categories

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

All
macOS
task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
relnote-firefox --- 70+
firefox70 + fixed

People

(Reporter: mstange, Assigned: mstange)

References

(Depends on 1 open bug, Blocks 1 open bug, Regressed 2 open bugs)

Details

Attachments

(1 file)

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

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.

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.

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.

Blocks: 1375298

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.

Depends on: 1575070

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.

Depends on: 1575419

Filed bug 1575419 for the test failure.

Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/eeeb0858e706
Enable CoreAnimation by default. r=jrmuizel
Depends on: 1514060

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.

Flags: needinfo?(mstange)
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/152bfc05a509
Enable CoreAnimation by default. r=jrmuizel
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Regressions: 1576113
Blocks: 1576390

Hi it seems that this is the cause of bug 1576399

Regressions: 1576483
Regressions: 1576399

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

relnote-firefox: --- → ?
Flags: needinfo?(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?

Flags: needinfo?(mstange)

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

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.

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

Flags: needinfo?(lhenry)

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!

Flags: needinfo?(lhenry)

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)

Regressions: 1581081
Regressions: 1586627
No longer regressions: 1586627
Regressions: 1588747
Blocks: 1305656
Blocks: 1569678
Regressions: 1592188
Regressions: 1591392
Regressions: 1579664
You need to log in before you can comment on or make changes to this bug.