GetVSyncParameters only once every 5 seconds on Linux

ui/gl/gl_surface_glx.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 extern "C" {
 #include <X11/Xlib.h>
 }
 
 #include "ui/gl/gl_surface_glx.h"
 
@@ skipping 38 matching lines @@
 bool g_glx_texture_from_pixmap_supported = false;
 bool g_glx_oml_sync_control_supported = false;
 
 // Track support of glXGetMscRateOML separately from GLX_OML_sync_control as a
 // whole since on some platforms (e.g. crosbug.com/34585), glXGetMscRateOML
 // always fails even though GLX_OML_sync_control is reported as being supported.
 bool g_glx_get_msc_rate_oml_supported = false;
 
 bool g_glx_sgi_video_sync_supported = false;
 
+#if defined(OS_LINUX)

[Ken Russell (switch to Gerrit), 2014/09/11 18:37:34]

Are the OS_LINUX #ifdefs really necessary? This entire file should mainly
(only?) be used on Linux -- is Chrome OS the other consumer?

Could a different constant be chosen on Linux vs. non-Linux? See below.

[brianderson, 2014/09/11 18:50:29]

I was under the impression the CrOS used this too. Trying to verify.

+// See crbug.com/373489
+// On Linux, querying the vsync parameters might burn CPU for up to an
+// entire vsync, so we only query periodically to reduce CPU usage.
+// 5 seconds is chosen somewhat abitrarily as a balance between:
+//  a) Drift in the phase of our signal.
+//  b) Potential janks from periodically pegging the CPU.
+static const base::TimeDelta kGetVSyncParametersMinPeriod =
+    base::TimeDelta::FromSeconds(5);
+#endif
+
 class OMLSyncControlVSyncProvider
     : public gfx::SyncControlVSyncProvider {
  public:
   explicit OMLSyncControlVSyncProvider(gfx::AcceleratedWidget window)
       : SyncControlVSyncProvider(),
         window_(window) {
   }
 
   virtual ~OMLSyncControlVSyncProvider() { }
 
@@ skipping 114 matching lines @@
   }
 
   void GetVSyncParameters(const VSyncProvider::UpdateVSyncCallback& callback) {
     base::TimeTicks now;
     {
       // Don't allow |window_| destruction while we're probing vsync.
       base::AutoLock locked(vsync_lock_);
 
       if (!context_ || cancel_vsync_flag_.IsSet())
         return;
-
       glXMakeCurrent(display_, window_, context_);
 
       unsigned int retrace_count = 0;
       if (glXWaitVideoSyncSGI(1, 0, &retrace_count) != 0)
         return;
 
       TRACE_EVENT_INSTANT0("gpu", "vblank", TRACE_EVENT_SCOPE_THREAD);
       now = base::TimeTicks::HighResNow();
 
       glXMakeCurrent(display_, 0, 0);
@@ skipping 46 matching lines @@
     }
 
     // Hand-off |shim_| to be deleted on the |vsync_thread_|.
     vsync_thread_->message_loop()->DeleteSoon(
         FROM_HERE,
         shim_.release());
   }
 
   virtual void GetVSyncParameters(
       const VSyncProvider::UpdateVSyncCallback& callback) OVERRIDE {
+#if defined(OS_LINUX)

[Ken Russell (switch to Gerrit), 2014/09/11 18:37:34]

Instead of an #ifdef here, could this be written like:
  if (kGetVsyncParametersMinPeriod > 0) {
    ...
  }

and set it to 0 if it shouldn't be applied?

[brianderson, 2014/09/11 18:50:29]

Done.

+    base::TimeTicks now = base::TimeTicks::Now();
+    if ((now - last_get_vsync_parameters_time_) < kGetVSyncParametersMinPeriod)
+      return;
+    last_get_vsync_parameters_time_ = now;
+#endif
+
     // Only one outstanding request per surface.
     if (!pending_callback_) {
       pending_callback_.reset(
           new VSyncProvider::UpdateVSyncCallback(callback));
       vsync_thread_->message_loop()->PostTask(
           FROM_HERE,
           base::Bind(&SGIVideoSyncProviderThreadShim::GetVSyncParameters,
                      base::Unretained(shim_.get()),
                      base::Bind(
                          &SGIVideoSyncVSyncProvider::PendingCallbackRunner,
@@ skipping 15 matching lines @@
   scoped_ptr<SGIVideoSyncProviderThreadShim> shim_;
 
   scoped_ptr<VSyncProvider::UpdateVSyncCallback> pending_callback_;
 
   // Raw pointers to sync primitives owned by the shim_.
   // These will only be referenced before we post a task to destroy
   // the shim_, so they are safe to access.
   base::CancellationFlag* cancel_vsync_flag_;
   base::Lock* vsync_lock_;
 
+#if defined(OS_LINUX)
+  base::TimeTicks last_get_vsync_parameters_time_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(SGIVideoSyncVSyncProvider);
 };
 
 SGIVideoSyncThread* SGIVideoSyncThread::g_video_sync_thread = NULL;
 
 // In order to take advantage of GLX_SGI_video_sync, we need a display
 // for use on a separate thread. We must allocate this before the sandbox
 // goes up (rather than on-demand when we start the thread).
 Display* SGIVideoSyncProviderThreadShim::display_ = NULL;
 
@@ skipping 361 matching lines @@
 
 void* PbufferGLSurfaceGLX::GetConfig() {
   return config_;
 }
 
 PbufferGLSurfaceGLX::~PbufferGLSurfaceGLX() {
   Destroy();
 }
 
 }  // namespace gfx