Closed Bug 1263053 Opened 8 years ago Closed 8 years ago

BasicCompositor on OS X is using sync transactions

Categories

(Core :: Graphics: Layers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- wontfix
firefox49 --- fixed
relnote-firefox --- 49+

People

(Reporter: mstange, Assigned: sotaro)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
:mstange, is it BasicCompositor specific problem?
Flags: needinfo?(mstange)
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: nobody → sotaro.ikeda.g
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+
Yea, I am going to update the patch.
Apply the comment. Carry "r=nical".
Attachment #8741244 - Attachment is obsolete: true
Attachment #8741603 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/70ec09a17094
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
This caused regression bug 1265873.
Blocks: 1265873
Flags: needinfo?(sotaro.ikeda.g)
No longer blocks: 1265873
Depends on: 1265873
Status: RESOLVED → REOPENED
Flags: needinfo?(sotaro.ikeda.g)
Resolution: FIXED → ---
Keywords: leave-open
: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.
Flags: needinfo?(bgirard)
We're not running Talos with BasicCompositor on OS X.
It seems better to backout to prevent regression.
Blocks: 1267569
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/3bc29971aa0a
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
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"
Milan, do we have numbers about that?
Are we talking about 2% or 20?
thanks
Flags: needinfo?(milan)
By comment 15, it was backed out from firefox 48.
But it's still in 49, right?
Flags: needinfo?(milan) → needinfo?(sotaro.ikeda.g)
(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.