Closed Bug 1547805 Opened 5 months ago Closed 5 months ago

Pref advanced layers back on

Categories

(Core :: Graphics: Layers, task, P3)

task

Tracking

()

VERIFIED FIXED
mozilla68
Tracking Status
firefox68 --- verified

People

(Reporter: rhunt, Assigned: rhunt)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file)

Unfortunately, no one has time to understand the reported regressions for removing advanced layers at this time.

So to be conservative we will pref it back on until we have more time to figure this out.

What are the regressions? Just the motionmark raptor test?

There are two regressions blocking bug 1544538.

Bug 1545380:
9% raptor-motionmark-animometer-firefox windows7-32-shippable opt 36.98 -> 33.67
7% tsvgx windows10-64-shippable opt e10s stylo 119.85 -> 128.18

Bug 1545313:
"Screen flashes at browser startup. The screen goes black for a moment.
This is noticeable if browser is maximized."

I'm not super concerned about the talos regressions, but the screen flashing seems to be of concern.

We still have a small number of users using the D3D11 compositor, even with Advanced Layers enabled. Have they had the black flashing screen the entire time?

(In reply to Matt Woodrow (:mattwoodrow) from comment #4)

We still have a small number of users using the D3D11 compositor, even with
Advanced Layers enabled. Have they had the black flashing screen the entire
time?

That's a good point. According to Andrew in bug 1545313 comment 4, he was able to reproduce the issue and it was only on CompositorD3D11. That would lead me to think that it's actually an existing issue.

CompositorD3D11 is only on 2% of the users, so maybe that's why we haven't had it reported. I'd be curious if it was regressed by something in the meantime since we turned on AL.

(In reply to Ryan Hunt [:rhunt] from comment #5)

(In reply to Matt Woodrow (:mattwoodrow) from comment #4)

We still have a small number of users using the D3D11 compositor, even with
Advanced Layers enabled. Have they had the black flashing screen the entire
time?

That's a good point. According to Andrew in bug 1545313 comment 4, he was able to reproduce the issue and it was only on CompositorD3D11. That would lead me to think that it's actually an existing issue.

CompositorD3D11 is only on 2% of the users, so maybe that's why we haven't had it reported. I'd be curious if it was regressed by something in the meantime since we turned on AL.

It brings up the general point I think that I'm not sure we want to have D3D11 around for such a small number of users. A part of me wants to just remove it, another part of me realizes that would mean we're -definitely- stuck on Advanced Layers though.

I definitely agree that we should only have one.

The biggest downside of AdvancedLayers is that is by far the largest backend, since it has its own LayerManager implementation as well as everything underneath it.

CompositorD3D11 is much simpler for maintenance, since it's basically identical to CompositorOGL, except with the API calls more or less directly translated.

For the purposes of minimizing our maintance overhead, so that maximum progress can be made on getting WebRender shipped to as many users as possible, then I think dropping AdvancedLayers is still the best way forward.

It might be a temporary performance regression for some users (though there's a real lack of evidence of real-world differences), but I think that's something we could live with.

Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/autoland/rev/b5867ca37387
Pref advanced layers back on. r=jrmuizel
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

(In reply to Matt Woodrow (:mattwoodrow) from comment #7)

I definitely agree that we should only have one.

The biggest downside of AdvancedLayers is that is by far the largest backend, since it has its own LayerManager implementation as well as everything underneath it.

CompositorD3D11 is much simpler for maintenance, since it's basically identical to CompositorOGL, except with the API calls more or less directly translated.

For the purposes of minimizing our maintance overhead, so that maximum progress can be made on getting WebRender shipped to as many users as possible, then I think dropping AdvancedLayers is still the best way forward.

It might be a temporary performance regression for some users (though there's a real lack of evidence of real-world differences), but I think that's something we could live with.

I think it's important to understand how little of a performance regression we can accept for Intel users considering our performance goals, and having just shipped the double buffering change to Intel users the Advanced Layers performance on my laptop is now back to being significantly better than WebRender's, so we might be stuck using it for a while.

Advanced Layers doesn't really need to be touched, it feels to me like the best idea may just be to let it sit there and do its job, not touching it too much. If we have indications CompositorD3D11 has bugs, lets kill it.

Regressions: 1548778

Thank you very much for reconsidering this change and enabling Advanced Layers back again! \o/

Feels much snappier and smoother (but it's subjective thing and maybe even placebo), but synthetic benchmark tests show 5-15% improvement with enabled Advanced Layers.

I hope that Advanced Layers won't be disabled and removed until WebRender will be enabled in Firefox stable builds and I can using it without enabling it manually and testing in Mozilla Firefox Nightly per information in about:support:

Decision Log
WEBRENDER
opt-in by default: WebRender is an opt-in feature
WEBRENDER_QUALIFIED
blocked by env: No qualified hardware

Status: RESOLVED → VERIFIED
Keywords: perf
QA Contact: Virtual

== Change summary for alert #20748 (as of Wed, 01 May 2019 22:40:01 GMT) ==

Improvements:

9% raptor-motionmark-animometer-firefox windows7-32-shippable opt 34.10 -> 37.27
3% raptor-motionmark-htmlsuite-firefox windows10-64-shippable opt 31.53 -> 32.62

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20748

You need to log in before you can comment on or make changes to this bug.