gpu: Reuse transfer buffers more aggresively

gpu/command_buffer/service/query_manager.cc

Index: gpu/command_buffer/service/query_manager.cc
diff --git a/gpu/command_buffer/service/query_manager.cc b/gpu/command_buffer/service/query_manager.cc
index 85e273c6c3a02b570cc87b634823c577eaaf63a8..c8ccace5cab45dc08b8a41a78f4b02e6afd45c8d 100644
--- a/gpu/command_buffer/service/query_manager.cc
+++ b/gpu/command_buffer/service/query_manager.cc
@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "gpu/command_buffer/common/gles2_cmd_format.h"
 #include "gpu/command_buffer/service/async_pixel_transfer_manager.h"
+#include "gpu/command_buffer/service/cmd_buffer_engine.h"
 #include "gpu/command_buffer/service/error_state.h"
 #include "gpu/command_buffer/service/feature_info.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
@@ -24,8 +25,15 @@ namespace {
 class AsyncPixelTransferCompletionObserverImpl
     : public AsyncPixelTransferCompletionObserver {
  public:
-  AsyncPixelTransferCompletionObserverImpl(uint32 submit_count)
-      : submit_count_(submit_count),
+  AsyncPixelTransferCompletionObserverImpl(
+      base::WeakPtr<QueryManager> manager,
+      scoped_refptr<base::MessageLoopProxy> proxy,
+      uint32 submit_count,
+      uint32 serial)
+      : manager_(manager),
+        proxy_(proxy),
+        submit_count_(submit_count),
+        serial_(serial),
         cancelled_(false) {}
 
   void Cancel() {
@@ -46,13 +54,31 @@ class AsyncPixelTransferCompletionObserverImpl
       // submit_count was written to sync->process_count.
       base::subtle::MemoryBarrier();
       sync->process_count = submit_count_;
+
+      if (serial_) {
+        proxy_->PostTask(
+            FROM_HERE,
+            base::Bind(&AsyncPixelTransferCompletionObserverImpl::SetSerial,
+                       this));

[reveman, 2014/01/11 23:39:04]

Hm, I think this will be a performance problem. We used to post tasks  back to
the main thread but we switched to writing directly to the shared memory from
the transfer thread instead. Two reason for doing this:

1. The main gpu thread might be busy so it can take awhile for it to process the
task.

2. And I think even more importantly, posting a task after each transfer results
in a lot of context switching overhead. This is because the transfer thread has
lower priority so posting a task to the main thread will consistently cause the
OS to switch to that thread and process the task.

I think we need to write directly to some shared memory from here instead of
posting a task.

[jadahl, 2014/01/12 09:52:24]

This is what I did initially, but changed to posting as I hadn't done it in a
thread safe manner. Is there a way to write to the command buffer state safely
directly from the upload thread?

[reveman, 2014/01/12 19:29:46]

You'll probably need to add a mutex to do that safely.

FYI, if we go the async token route then we'd want to handle this at the decoder
level instead.

+      }
     }
   }
 
  private:
   virtual ~AsyncPixelTransferCompletionObserverImpl() {}
 
+  void SetSerial() {
+    if (!manager_.get())
+      return;
+
+    manager_->decoder()->engine()->set_serial(serial_);

[piman, 2014/01/11 02:02:32]

So, the rest of the logic assumes serials get set on the CommandBuffer in the
same order they are created. Is it currently true? (assumes queries always
complete in exactly the same order as they are created). How can we defend for
that in the future, if someone wants to extend the serial to other queries?

[jadahl, 2014/01/11 11:35:29]

AFAIK uploads complete in order, so queries should as well, just as this
function should be invoked in order as such. Usage parallel to async uploads is
problematic and somewhat of a limitation of this approach, as they would have to
follow the order of uploads.

+  }
+
+  base::WeakPtr<QueryManager> manager_;
+  scoped_refptr<base::MessageLoopProxy> proxy_;
+
   uint32 submit_count_;
+  uint32 serial_;
 
   base::Lock lock_;
   bool cancelled_;
@@ -99,7 +125,10 @@ bool AsyncPixelTransfersCompletedQuery::End(uint32 submit_count) {
   mem_params.shm_data_offset = shm_offset();
   mem_params.shm_data_size = sizeof(QuerySync);
 
-  observer_ = new AsyncPixelTransferCompletionObserverImpl(submit_count);
+  scoped_refptr<base::MessageLoopProxy> proxy =
+      base::MessageLoop::current()->message_loop_proxy();
+  observer_ = new AsyncPixelTransferCompletionObserverImpl(
+      manager()->AsWeakPtr(), proxy, submit_count, serial());
 
   // Ask AsyncPixelTransferDelegate to run completion callback after all
   // previous async transfers are done. No guarantee that callback is run