Reduce internal Flush() in GL resource glGen/Delete APIs.

gpu/command_buffer/client/share_group.cc

Index: gpu/command_buffer/client/share_group.cc
diff --git a/gpu/command_buffer/client/share_group.cc b/gpu/command_buffer/client/share_group.cc
index 8711ce817e64485ac76cf65115bb737be88bc8b3..1238791ca3365be73d9b22a3d4538fcba5c791fd 100644
--- a/gpu/command_buffer/client/share_group.cc
+++ b/gpu/command_buffer/client/share_group.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <vector>
+
 #include "gpu/command_buffer/client/share_group.h"
 
 #include "base/logging.h"
@@ -50,6 +52,8 @@ class IdHandler : public IdHandlerInterface {
     (gl_impl->*delete_fn)(n, ids);
     // We need to ensure that the delete call is evaluated on the service side
     // before any other contexts issue commands using these client ids.
+    // TODO(vmiura): Can remove this by virtualizing internal ids, however
+    // this code only affects PPAPI for now.
     gl_impl->helper()->CommandBufferHelper::Flush();
     return true;
   }
@@ -70,9 +74,44 @@ class IdHandler : public IdHandlerInterface {
 // An id handler that requires Gen before Bind.
 class StrictIdHandler : public IdHandler {

[piman, 2014/02/12 23:52:34]

nit: at this point, there's little advantage to inherit from IdHandler, rather
inherit from IdHandlerInterface?
That way, IdHandler fields can become private.

[vmiura, 2014/02/14 00:11:11]

Done.

  public:
-  StrictIdHandler() {}
+  StrictIdHandler() : pending_flush_generation_(0) {}
   virtual ~StrictIdHandler() {}
 
+  // Overridden from IdHandlerInterface.
+  virtual void MakeIds(GLES2Implementation* gl_impl,
+                       GLuint id_offset,
+                       GLsizei n,
+                       GLuint* ids) OVERRIDE {
+    base::AutoLock auto_lock(lock_);
+
+    // Collect pending FreeIds from other flush_generation.
+    CollectPendingFreeIds(gl_impl);
+
+    // Allocate Ids.
+    for (GLsizei ii = 0; ii < n; ++ii) {
+      ids[ii] = id_allocator_.AllocateID();
+    }
+  }
+
+  // Overridden from IdHandlerInterface.
+  virtual bool FreeIds(GLES2Implementation* gl_impl,
+                       GLsizei n,
+                       const GLuint* ids,
+                       DeleteFn delete_fn) OVERRIDE {
+    base::AutoLock auto_lock(lock_);
+
+    // Collect pending FreeIds from other flush_generation.
+    CollectPendingFreeIds(gl_impl);
+
+    // Save Ids to free in a later flush_generation.
+    for (GLsizei ii = 0; ii < n; ++ii) {
+      pending_free_ids_.push_back(ids[ii]);
+    }
+
+    (gl_impl->*delete_fn)(n, ids);
+    return true;
+  }
+
   // Overridden from IdHandler.
   virtual bool MarkAsUsedForBind(GLuint id) OVERRIDE {
 #ifndef NDEBUG
@@ -83,6 +122,21 @@ class StrictIdHandler : public IdHandler {
 #endif
     return true;
   }
+
+ private:
+  void CollectPendingFreeIds(GLES2Implementation* gl_impl) {
+    uint32 flush_generation = gl_impl->helper()->flush_generation();
+    if (pending_flush_generation_ != flush_generation) {

[piman, 2014/02/12 23:52:34]

gl_impl->helper()->flush_generation() is per-command buffer (i.e. per context),
whereas |this|, hence |pending_flush_generation_| is per share group, so it
doesn't really make sense to compare those (it'll do the wrong thing).

I suppose you could keep a per-command buffer {pending_free_ids_,
pending_flush_generation_} struct...

There's another issue which is that you only collect on subsequent Gen/Delete,
which means that, for example, if a command buffer gets destroyed, we won't ever
reclaim the ids that were last deleted.

[vmiura, 2014/02/13 00:20:17]

Great point.

I think this pending free handling can move up into GLES2Implementation.

[vmiura, 2014/02/14 00:11:11]

Moved these into per-GLES2Implementation data, but same idea.

+      pending_flush_generation_ = flush_generation;
+      for (uint32 ii = 0; ii < pending_free_ids_.size(); ++ii) {
+        id_allocator_.FreeID(pending_free_ids_[ii]);
+      }
+      pending_free_ids_.clear();
+    }
+  }
+
+  std::vector<GLuint> pending_free_ids_;
+  uint32 pending_flush_generation_;
 };
 
 // An id handler for ids that are never reused.