After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779039 - Add a threaded implementation of waiting for swap events
Add a threaded implementation of waiting for swap events
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
: 768190 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-02-21 21:24 UTC by Owen Taylor
Modified: 2018-03-09 19:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
CoglGPUInfo - fix check for NVIDIA (1009 bytes, patch)
2017-02-21 21:25 UTC, Owen Taylor
committed Details | Review
CoglWinsysGLX: factor out some duplicated code (2.67 KB, patch)
2017-02-21 21:25 UTC, Owen Taylor
committed Details | Review
Usability of SGI_video_sync is per-display not per-renderer (7.23 KB, patch)
2017-02-21 21:25 UTC, Owen Taylor
committed Details | Review
Fix the get_clock_time() without GLX_OML_sync_control (1.14 KB, patch)
2017-02-21 21:25 UTC, Owen Taylor
committed Details | Review
For NVIDIA proprietary drivers, implement sync events with a thread (13.99 KB, patch)
2017-02-21 21:25 UTC, Owen Taylor
committed Details | Review
Add cogl_xlib_renderer_set_threaded_swap_wait_enabled() (4.19 KB, patch)
2017-02-21 21:25 UTC, Owen Taylor
committed Details | Review
Call cogl_xlib_renderer_set_threaded_swap_wait_enabled() (2.35 KB, patch)
2017-02-21 21:25 UTC, Owen Taylor
committed Details | Review

Description Owen Taylor 2017-02-21 21:24:58 UTC
With the NVIDIA binary driver, the lack of the INTEL_swap_event GLX
extension means that if Mutter is continually drawing frames, it will
always block in glXSwapBuffers(), rather than going idle. This
means that if code in GNOME Shell or other libmutter clients adds a
main loop idle function, it will never be called. To avoid having a trap
of different behaviors with different drivers, add an implementation of
waiting for VBlank that uses a separate thread, and enable it only if
running with the NVIDIA drivers.

The primary bug this fixes is that GNOME Shell is restarted manually
or automatically because a stereo client starts, it hangs up on restart
until there is a pause in rendering, so (with a stereo system) if you do
'glxgears -stereo' and let GNOME Shell restart to handle it, you get a
hang. But there are quite a few uses of idles in GNOME Shell, so there are
likely other similar bugs lurking.

I wrote this code for GNOME 3.14 and it's in RHEL 7.3; there were no
substantial changes in porting it to master, other than eliminating the
API I added in libclutter, since GNOME Shell now talks to Cogl directly
at setup time.
Comment 1 Owen Taylor 2017-02-21 21:25:03 UTC
Created attachment 346401 [details] [review]
CoglGPUInfo - fix check for NVIDIA

NVIDIA drivers have a vendor of "NVIDIA Corporation" not "NVIDIA".
Check for both in case older drivers did use "NVIDIA"
Comment 2 Owen Taylor 2017-02-21 21:25:08 UTC
Created attachment 346402 [details] [review]
CoglWinsysGLX: factor out some duplicated code

Add a helper function for repeated calls to clock_gettime(CLOCK_MONOTONIC)
Comment 3 Owen Taylor 2017-02-21 21:25:13 UTC
Created attachment 346403 [details] [review]
Usability of SGI_video_sync is per-display not per-renderer

As previously commented in the code, SGI_video_sync is per-display, rather
than per-renderer. The is_direct flag for the renderer was tested before
it was initialized (per-display) and that resulted in SGI_video_sync
never being used.
Comment 4 Owen Taylor 2017-02-21 21:25:18 UTC
Created attachment 346404 [details] [review]
Fix the get_clock_time() without GLX_OML_sync_control

When we don't have GLX_OML_sync_control, we still can set the
frame presentation time, but we always use the system monotonic time,
so return that from get_clock_time().
Comment 5 Owen Taylor 2017-02-21 21:25:23 UTC
Created attachment 346405 [details] [review]
For NVIDIA proprietary drivers, implement sync events with a thread

It's a good guess that the buffer swap will occur at the next vblank,
so use glXWaitVideoSync in a separate thread to deliver a sync event
rather than just letting the client block when frame drawing, which
can signficantly change app logic as compared to the INTEL_swap_event
case.
Comment 6 Owen Taylor 2017-02-21 21:25:28 UTC
Created attachment 346406 [details] [review]
Add cogl_xlib_renderer_set_threaded_swap_wait_enabled()

Because the threaded-swap-wait functionality requires XInitThreads(),
and because it isn't clear that it is a win for all applications,
add a API function to conditionally enable it.

Fix the cogl-crate example not to just have a hard-coded dependency
on libX11.
Comment 7 Owen Taylor 2017-02-21 21:25:33 UTC
Created attachment 346407 [details] [review]
Call cogl_xlib_renderer_set_threaded_swap_wait_enabled()

Set up things so that if the INTEL_swap_event extension is not present,
but the driver is known to have good thread support, we use an extra
thread and call glXWaitVideoSync() in the thread. This allows idles
to work properly, even when Mutter is constantly redrawing new frames;
otherwise, without INTEL_swap_event, we'll just block in glXSwapBuffers().
Comment 8 Rui Matos 2017-02-22 12:52:01 UTC
Just noting that I reviewed the standalone cogl version of this in bug 768190 so IMO it can land in master.
Comment 9 Owen Taylor 2017-02-22 16:13:48 UTC
*** Bug 768190 has been marked as a duplicate of this bug. ***
Comment 10 Owen Taylor 2017-02-22 16:15:14 UTC
Attachment 346401 [details] pushed as a9f139c - CoglGPUInfo - fix check for NVIDIA
Attachment 346402 [details] pushed as 1171c4f - CoglWinsysGLX: factor out some duplicated code
Attachment 346403 [details] pushed as 1b03dd6 - Usability of SGI_video_sync is per-display not per-renderer
Attachment 346404 [details] pushed as e078838 - Fix the get_clock_time() without GLX_OML_sync_control
Attachment 346405 [details] pushed as 690b232 - For NVIDIA proprietary drivers, implement sync events with a thread
Attachment 346406 [details] pushed as d200868 - Add cogl_xlib_renderer_set_threaded_swap_wait_enabled()
Attachment 346407 [details] pushed as 383ba56 - Call cogl_xlib_renderer_set_threaded_swap_wait_enabled()
Comment 11 Hussam Al-Tayeb 2017-02-22 18:56:04 UTC
I think they is making mutter constantly use 20% CPU (corei5 processor) on my nvidia card.
Comment 13 Jeremy Bicha 2017-03-03 17:10:50 UTC
Also it broke Budgie: https://bugzilla.gnome.org/779538
Comment 14 Maxim 2017-04-11 22:03:17 UTC
(In reply to Hussam Al-Tayeb from comment #12)
> https://git.gnome.org/browse/mutter/commit/
> ?id=d20086845981c7a41e89d781bac3e07c59cd6037 is ok.
> 
> https://git.gnome.org/browse/mutter/commit/
> ?id=383ba566bd7c2a76d0856015a66e47caedef06b6 causes high CPU usage.

I can confirm this. I make a screenshot of cpu usage before and after  applying  
Call-coglxlibrenderersetthreadedswapwaitenabled.patch

http://storage4.static.itmages.ru/i/17/0411/h_1491948046_9928347_1598f98b62.png
Comment 15 Maxim 2017-04-17 11:47:00 UTC
I also checked and I noticed that these changes have improved responsiveness when moving windows. But now, the animations of the Gnome Shell to lag even more. Now without the load on the CPU, the decrease in FPS became visible. This is all tested on a proprietary Nvidia + 650GTX.