BasicCompositor on OS X is using sync transactions

RESOLVED FIXED in Firefox 49

Status

()

Core
Graphics: Layers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mstange, Assigned: sotaro)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 wontfix, firefox49 fixed, relnote-firefox 49+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Sync transactions are bad.

I noticed this on https://www.just-eat.ca/restaurants-casa-di-giorgio-queen-east/menu?opt=0a765732 because the sidebars were jittering much more than they need to.
(Assignee)

Comment 1

2 years ago
:mstange, is it BasicCompositor specific problem?
Flags: needinfo?(mstange)
(Reporter)

Comment 2

2 years ago
Yes. It looks like we're not using tiles when BasicCompositor is used. Using tiles might fix both this bug and bug 1262863.
Flags: needinfo?(mstange)
(Assignee)

Updated

2 years ago
Assignee: nobody → sotaro.ikeda.g
(Assignee)

Comment 3

2 years ago
Created attachment 8741244 [details] [diff] [review]
patch - Enable tiling on BasicCompositor
(Assignee)

Updated

2 years ago
Attachment #8741244 - Flags: review?(nical.bugzilla)
Comment on attachment 8741244 [details] [diff] [review]
patch - Enable tiling on BasicCompositor

Review of attachment 8741244 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/client/ClientPaintedLayer.cpp
@@ +164,5 @@
>  #ifndef MOZ_X11
>        && (AsShadowForwarder()->GetCompositorBackendType() == LayersBackend::LAYERS_OPENGL ||
>            AsShadowForwarder()->GetCompositorBackendType() == LayersBackend::LAYERS_D3D9 ||
> +          AsShadowForwarder()->GetCompositorBackendType() == LayersBackend::LAYERS_D3D11 ||
> +          AsShadowForwarder()->GetCompositorBackendType() == LayersBackend::LAYERS_BASIC)

That's about all of the backends so you can just remove the whole #ifndef block!
Attachment #8741244 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 5

2 years ago
Yea, I am going to update the patch.
(Assignee)

Comment 6

2 years ago
Created attachment 8741603 [details] [diff] [review]
patch - Enable tiling on BasicCompositor

Apply the comment. Carry "r=nical".
Attachment #8741244 - Attachment is obsolete: true
Attachment #8741603 - Flags: review+

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/70ec09a17094
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
This caused regression bug 1265873.
Blocks: 1265873

Updated

2 years ago
Flags: needinfo?(sotaro.ikeda.g)

Updated

2 years ago
No longer blocks: 1265873
Depends on: 1265873
(Assignee)

Updated

2 years ago
Status: RESOLVED → REOPENED
Flags: needinfo?(sotaro.ikeda.g)
Resolution: FIXED → ---
(Assignee)

Updated

2 years ago
Keywords: leave-open
(Assignee)

Comment 10

2 years ago
:BenWa, is it better to back out until bug 1265873 is addressed?
Flags: needinfo?(bgirard)
We're uplifting this coming Monday so I think it is better to back this out and reland with the fix at the start of the next cycle. Unless we measured a good performance win for this patch but I don't see any Talos improvement notices.
status-firefox48: fixed → ?
Flags: needinfo?(bgirard)
(Reporter)

Comment 12

2 years ago
We're not running Talos with BasicCompositor on OS X.
(Assignee)

Comment 13

2 years ago
It seems better to backout to prevent regression.

Updated

2 years ago
Blocks: 1267569
(Assignee)

Updated

2 years ago
Keywords: leave-open

Comment 18

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3bc29971aa0a
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Release Note Request (optional, but appreciated)
[Why is this notable]:
[Suggested wording]: Improve overall performance on OS X systems without hardware acceleration.
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
49+ for relnote - added "Improve overall performance on OS X systems without hardware acceleration"
relnote-firefox: ? → 49+
Milan, do we have numbers about that?
Are we talking about 2% or 20?
thanks
Flags: needinfo?(milan)
(Assignee)

Comment 22

2 years ago
By comment 15, it was backed out from firefox 48.
status-firefox48: ? → wontfix
But it's still in 49, right?
Flags: needinfo?(milan) → needinfo?(sotaro.ikeda.g)
(Assignee)

Comment 24

2 years ago
(In reply to Milan Sreckovic [:milan] from comment #23)
> But it's still in 49, right?

Yes. It was checked in again by Comment 18.
Flags: needinfo?(sotaro.ikeda.g)
You need to log in before you can comment on or make changes to this bug.