cc: Avoid issuing COMMANDS_COMPLETED queries unless necessary.

cc/output/gl_renderer.cc

 // Copyright 2010 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.
 
 #include "cc/output/gl_renderer.h"
 
 #include <algorithm>
 #include <limits>
 #include <set>
 #include <string>
@@ skipping 45 matching lines @@
 #include "ui/gfx/rect_conversions.h"
 
 using gpu::gles2::GLES2Interface;
 
 namespace cc {
 namespace {
 
 class FallbackFence : public ResourceProvider::Fence {
  public:
   explicit FallbackFence(gpu::gles2::GLES2Interface* gl)
-      : gl_(gl), has_passed_(false) {}
+      : gl_(gl), has_passed_(true) {}
 
   // Overridden from ResourceProvider::Fence:
+  virtual void Set() OVERRIDE { has_passed_ = false; }
   virtual bool HasPassed() OVERRIDE {
     if (!has_passed_) {
       has_passed_ = true;
       Synchronize();
     }
     return true;
   }
 
  private:
   virtual ~FallbackFence() {}
@@ skipping 149 matching lines @@
   base::CancelableClosure finished_read_pixels_callback;
   unsigned buffer;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PendingAsyncReadPixels);
 };
 
 class GLRenderer::SyncQuery {
  public:
   explicit SyncQuery(gpu::gles2::GLES2Interface* gl)
-      : gl_(gl), query_id_(0u), weak_ptr_factory_(this) {
+      : gl_(gl), query_id_(0u), is_pending_(false), weak_ptr_factory_(this) {
     gl_->GenQueriesEXT(1, &query_id_);
   }
   virtual ~SyncQuery() { gl_->DeleteQueriesEXT(1, &query_id_); }
 
   scoped_refptr<ResourceProvider::Fence> Begin() {
     DCHECK(!weak_ptr_factory_.HasWeakPtrs() || !IsPending());
     // Invalidate weak pointer held by old fence.
     weak_ptr_factory_.InvalidateWeakPtrs();
-    gl_->BeginQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM, query_id_);
+    is_pending_ = false;
     return make_scoped_refptr<ResourceProvider::Fence>(
         new Fence(weak_ptr_factory_.GetWeakPtr()));
   }
 
-  void End() { gl_->EndQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM); }
+  void Set() { is_pending_ = true; }
+
+  void End() {
+    if (!is_pending_)
+      return;
+
+    gl_->BeginQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM, query_id_);

[piman, 2014/07/18 22:19:42]

nit: it's a bit of an abuse. Maybe add a comment saying that BeginQueryEXT on
GL_COMMANDS_COMPLETED_CHROMIUM is effectively a noop relative to GL, so it
doesn't matter where it happens.

It's conceivable that in the future, for out-of-order GPUs (e.g. tilers), a more
optimal fence (with another extension) would need to bracket draw calls as to
not force ordering. Then we'd want to revisit this.

[reveman, 2014/07/18 23:51:06]

Good point. I've updated the patch so we're still issuing BeginQueryEXT before
any drawing commands that depend on the query (seems like a good idea to keep
that behavior if possible) and I've added some comments to make this more clear.

+    gl_->EndQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM);
+  }
 
   bool IsPending() {
-    unsigned available = 1;
+    if (!is_pending_)
+      return false;
+
+    unsigned result_available = 1;
     gl_->GetQueryObjectuivEXT(
-        query_id_, GL_QUERY_RESULT_AVAILABLE_EXT, &available);
-    return !available;
+        query_id_, GL_QUERY_RESULT_AVAILABLE_EXT, &result_available);
+    is_pending_ = !result_available;
+    return is_pending_;
   }
 
   void Wait() {
+    if (!is_pending_)
+      return;
+
     unsigned result = 0;
     gl_->GetQueryObjectuivEXT(query_id_, GL_QUERY_RESULT_EXT, &result);
   }
 
  private:
   class Fence : public ResourceProvider::Fence {
    public:
     explicit Fence(base::WeakPtr<GLRenderer::SyncQuery> query)
         : query_(query) {}
 
     // Overridden from ResourceProvider::Fence:
+    virtual void Set() OVERRIDE {
+      DCHECK(query_);
+      query_->Set();
+    }
     virtual bool HasPassed() OVERRIDE {
       return !query_ || !query_->IsPending();
     }
 
    private:
     virtual ~Fence() {}
 
     base::WeakPtr<SyncQuery> query_;
 
     DISALLOW_COPY_AND_ASSIGN(Fence);
   };
 
   gpu::gles2::GLES2Interface* gl_;
   unsigned query_id_;
+  bool is_pending_;
   base::WeakPtrFactory<SyncQuery> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncQuery);
 };
 
 scoped_ptr<GLRenderer> GLRenderer::Create(
     RendererClient* client,
     const LayerTreeSettings* settings,
     OutputSurface* output_surface,
     ResourceProvider* resource_provider,
@@ skipping 2874 matching lines @@
     context_support_->ScheduleOverlayPlane(
         overlay.plane_z_order,
         overlay.transform,
         pending_overlay_resources_.back()->texture_id(),
         overlay.display_rect,
         overlay.uv_rect);
   }
 }
 
 }  // namespace cc