ASAP mode blocks until VBlank when swapping buffers using GL layers on Linux

RESOLVED FIXED in Firefox 50

Status

()

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

People

(Reporter: acomminos, Assigned: acomminos)

Tracking

(Blocks: 2 bugs)

50 Branch
mozilla50
Unspecified
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Looks like tests time out more frequently with GL layers enabled; they don't appear to be hanging, however (hooray!). I'm betting things are just slower.

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=cafa7213ef23c20fd5774db249db63b07314dcd9&selectedJob=30273630
(Assignee)

Comment 1

2 years ago
Wow, looks like the talos numbers got shaken up quite a bit by that push;

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-inbound&originalRevision=85825c55516f39c0f3763db8036a47055791936a&newProject=mozilla-inbound&newRevision=cafa7213ef23c20fd5774db249db63b07314dcd9&framework=1&filter=linux

Something recent is likely to have regressed our performance with GL layers on Linux significantly; the numbers weren't this dismal the last time I did a push with talos jobs.

I wouldn't be surprised if this is related to these timeouts.
(Assignee)

Comment 2

2 years ago
Some investigation has revealed that the talos regressions are more than likely because our GLX buffer swaps block until the next VBlank, even under ASAP mode. The unit tests are more than likely running slower due to this as well.

Adding support for SGI_swap_control and calling glXSwapIntervalSGI(0) under ASAP mode should resolve this issue (like we do for CGL contexts in https://dxr.mozilla.org/mozilla-central/rev/5f95858f8ddf21ea2271a12810332efd09eff138/gfx/gl/GLContextProviderCGL.mm#122).
(Assignee)

Comment 3

2 years ago
Correction; we'll have to use MESA_swap_control or GLX_swap_control. SGI_swap_control prohibits an interval of zero, which is what we want in ASAP mode (no blocking).
(Assignee)

Comment 4

2 years ago
Created attachment 8763411 [details]
Bug 1280744 - Set swap interval to 0 when running in ASAP mode on GLX.

Review commit: https://reviewboard.mozilla.org/r/59574/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59574/
Attachment #8763411 - Flags: review?(jgilbert)
(Assignee)

Comment 5

2 years ago
This patch fixes many of the regressions with ASAP mode talos suites.

Comparison using GL layers with ASAP vblanks vs. without ASAP vblanks:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-inbound&originalRevision=cafa7213ef23&newProject=try&newRevision=2581d23f1890&framework=1&filter=linux

Comparison using GL layers with ASAP vblanks vs. basic composition:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=3ce53bd1e25b&newProject=try&newRevision=2581d23f1890&framework=1&filter=linux

Many of the remaining non-ASAP test regressions appear to be drawing related, such as tart- in which case, GL layers provides very little benefit.

We also get some additional wins from never waiting for vblanks (even with ASAP off), particularly in tresize;

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=2581d23f1890&newProject=try&newRevision=88178bb543f5&framework=1

Lastly, there are a few cases where e10s performance has regressed to be in line with non-e10s performance. This is worth some further investigation, as the e10s numbers appear to be suspiciously low with basic composition.
(Assignee)

Comment 6

2 years ago
I didn't realize this before, but the test slaves (i.e. tst-linux64-spot) are running llvmpipe, which I believe does not block on VBlank for buffer swaps. It's quite likely then that we're looking at two separate issues, the other possibly being caused by the additional slowness of software rendering. I'm going to repurpose this bug to fix ASAP mode on the talos machines.
Summary: More frequent test timeouts with GL layers enabled on Linux test slaves → ASAP mode blocks until VBlank when swapping buffers using GL layers on Linux
tracking-e10s: --- → ?

Updated

2 years ago
Blocks: 984139
tracking-e10s: ? → +
Comment on attachment 8763411 [details]
Bug 1280744 - Set swap interval to 0 when running in ASAP mode on GLX.

https://reviewboard.mozilla.org/r/59574/#review57748

It's fine, but these fixes would be nice.

::: gfx/gl/GLXLibrary.h:145
(Diff revision 1)
>      bool UseTextureFromPixmap() { return mUseTextureFromPixmap; }
>      bool HasRobustness() { return mHasRobustness; }
>      bool HasCreateContextAttribs() { return mHasCreateContextAttribs; }
>      bool SupportsTextureFromPixmap(gfxASurface* aSurface);
>      bool SupportsVideoSync();
> +    bool SupportsSwapControl();

bool SupportsSwapControl() const { return bool(xSwapIntervalInternal); }

::: gfx/gl/GLXLibrary.h:245
(Diff revision 1)
>      PFNGLXGETVIDEOSYNCSGI xGetVideoSyncInternal;
>  
>      typedef int (GLAPIENTRY *PFNGLXWAITVIDEOSYNCSGI) (int divisor, int remainder, unsigned int *count);
>      PFNGLXWAITVIDEOSYNCSGI xWaitVideoSyncInternal;
>  
> +    typedef void (GLAPIENTRY *PFNGLXSWAPINTERVALEXT) (Display *dpy, GLXDrawable drawable, int interval);

star to left

::: gfx/gl/GLXLibrary.h:260
(Diff revision 1)
>      bool mUseTextureFromPixmap;
>      bool mDebug;
>      bool mHasRobustness;
>      bool mHasCreateContextAttribs;
>      bool mHasVideoSync;
> +    bool mHasSwapControl;

We shouldn't need this member.
Attachment #8763411 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 8

2 years ago
Comment on attachment 8763411 [details]
Bug 1280744 - Set swap interval to 0 when running in ASAP mode on GLX.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59574/diff/1-2/

Comment 9

2 years ago
Pushed by acomminos@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c96cf7b1b9c
Set swap interval to 0 when running in ASAP mode on GLX. r=jgilbert

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3c96cf7b1b9c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.