gpu: Reuse transfer buffers more aggresively

gpu/command_buffer/service/query_manager.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.
 
 #include "gpu/command_buffer/service/query_manager.h"
 
 #include "base/atomicops.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/shared_memory.h"
 #include "base/synchronization/lock.h"
 #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"
 
 namespace gpu {
 namespace gles2 {
 
 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() {
     base::AutoLock locked(lock_);
     cancelled_ = true;
   }
 
   virtual void DidComplete(const AsyncMemoryParams& mem_params) OVERRIDE {
     base::AutoLock locked(lock_);
     if (!cancelled_) {
       DCHECK(mem_params.shared_memory);
       DCHECK(mem_params.shared_memory->memory());
       void* data = static_cast<int8*>(mem_params.shared_memory->memory()) +
                    mem_params.shm_data_offset;
       QuerySync* sync = static_cast<QuerySync*>(data);
 
       // Need a MemoryBarrier here to ensure that upload completed before
       // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(AsyncPixelTransferCompletionObserverImpl);
 };
 
 class AsyncPixelTransfersCompletedQuery
     : public QueryManager::Query,
       public base::SupportsWeakPtr<AsyncPixelTransfersCompletedQuery> {
@@ skipping 26 matching lines @@
   // Get the real shared memory since it might need to be duped to prevent
   // use-after-free of the memory.
   Buffer buffer = manager()->decoder()->GetSharedMemoryBuffer(shm_id());
   if (!buffer.shared_memory)
     return false;
   mem_params.shared_memory = buffer.shared_memory;
   mem_params.shm_size = buffer.size;
   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
   // on the current thread.
   manager()->decoder()->GetAsyncPixelTransferManager()
       ->AsyncNotifyCompletion(mem_params, observer_);
 
   return AddToPendingTransferQueue(submit_count);
 }
 
@@ skipping 558 matching lines @@
 bool QueryManager::EndQuery(Query* query, uint32 submit_count) {
   DCHECK(query);
   if (!RemovePendingQuery(query)) {
     return false;
   }
   return query->End(submit_count);
 }
 
 }  // namespace gles2
 }  // namespace gpu