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 619516 - [perf] Add glx.swapComplete event
[perf] Add glx.swapComplete event
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-05-24 13:56 UTC by Owen Taylor
Modified: 2010-05-26 19:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[perf] Add glx.swapComplete event (3.07 KB, patch)
2010-05-24 13:56 UTC, Owen Taylor
reviewed Details | Review
[perf] Add glx.swapComplete event (3.81 KB, patch)
2010-05-26 18:31 UTC, Owen Taylor
committed Details | Review

Description Owen Taylor 2010-05-24 13:56:16 UTC
Add an event when we receive an event on buffer swap completion; we'll
only get this if Clutter is using the INTEL_swap_event GLX extension,
but it's useful to see the actual timing of video frames.

The recorded event includes the actual timestamp of the swap, since
we are given that in the GLX event - on my system it tends to be
consistently 80-100us before we log the event, but if something was
going wrong in event handling (too much synchronous work), then that
could could show up as a longer delay.
Comment 1 Owen Taylor 2010-05-24 13:56:19 UTC
Created attachment 161863 [details] [review]
[perf] Add glx.swapComplete event
Comment 2 drago01 2010-05-26 15:15:15 UTC
Review of attachment 161863 [details] [review]:

::: src/gnome-shell-plugin.c
@@ +470,3 @@
                                   XEvent       *xev)
 {
+#ifdef GLX_INTEL_swap_event

Wouldn't it make sense to check whether the extension is present at all too? 
(Of course only once and not for each event).
Comment 3 Owen Taylor 2010-05-26 18:25:12 UTC
Hmm. 

Yes, and no.

GLX event IDs are permanently allocated, so if we get an event with event_type == (glx event base> + 15) that's a swap event, always.

*Unless* glx_event_base + 15 is outside of the range of event ID's that the GLX X extension has reserved.

It's unclear to me the exact history of what range of events were being reserved, but it's certainly conceptually possible that the server might not be reserving that many events for an older server that doesn't support INTEL_swap_event.

So, probably, yes, we should check for the extension.
Comment 4 Owen Taylor 2010-05-26 18:31:50 UTC
Created attachment 162038 [details] [review]
[perf] Add glx.swapComplete event

This version checks for the extension, and also ignores swap
events with a ust (system time) field of 0 - this isn't allowed
by the spec, and I was seeing events sent that way with a
radeon driver version which has a partial/buggy implementation of
INTEL_swap_complete
Comment 5 drago01 2010-05-26 19:23:37 UTC
Review of attachment 162038 [details] [review]:

OK, this make sense and doing the check does not really hurt anyway.
Comment 6 Owen Taylor 2010-05-26 19:39:05 UTC
Attachment 162038 [details] pushed as 0eeb627 - [perf] Add glx.swapComplete event