gpu: Reuse transfer buffers more aggresively

gpu/command_buffer/service/async_pixel_transfer_manager.h

 // Copyright 2013 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.
 
 #ifndef GPU_COMMAND_BUFFER_SERVICE_ASYNC_PIXEL_TRANSFER_MANAGER_H_
 #define GPU_COMMAND_BUFFER_SERVICE_ASYNC_PIXEL_TRANSFER_MANAGER_H_
 
 #include <set>
 
 #include "base/basictypes.h"
@@ skipping 44 matching lines @@
     : public gles2::TextureManager::DestructionObserver {
  public:
   static AsyncPixelTransferManager* Create(gfx::GLContext* context);
 
   virtual ~AsyncPixelTransferManager();
 
   void Initialize(gles2::TextureManager* texture_manager);
 
   virtual void BindCompletedAsyncTransfers() = 0;
 
+  // Run the given callback when all already queued tasks has been processed.
+  // This includes asynchronous upload tasks.

[piman, 2014/02/07 22:58:20]

I'm not the biggest fan of this pattern, because it's very hard to reason with.
It's not inconceivable that an async upload needs to do several hops between
threads, in which case "AsyncRun" becomes hard to define.
I'd rather pass the callback to AsyncPixelTransferDelegate::AsyncTexImage2D with
the explicit semantic of "this callback is run as soon as the input buffer has
been consumed, possibly run on another thread", and have the implementation of
that semantic being the responsibility of each delegate (or manager).

[jadahl, 2014/02/08 09:18:25]

Sounds reasonable to pass the callback to AsyncTexImage2D, I'll update the patch
to do that.

[jadahl, 2014/02/09 11:54:52]

Reconsidering this, I think we still need something like this, because not every
AsyncTexImage2D command results in a call to Manager::AsyncTexImage2D (in case
of error), and we cannot write that upload token directly, but we don't want to
drop it because it'll block free:ing the memory until some successful upload.

+  virtual void AsyncRun(const base::Closure& callback) = 0;

[epennerAtGoogle, 2014/02/07 20:43:58]

This leads to duplicate work happening here where we notify the query and also
update the token. It's fine for this patch, but do you think we could use the
token in the client eventually to replace the queries being used?

[jadahl, 2014/02/08 09:18:25]

I'm thinking, can't we just handle those queries completely on the client side
after this?

+
   // There's no guarantee that callback will run on the caller thread.
   virtual void AsyncNotifyCompletion(
       const AsyncMemoryParams& mem_params,
       AsyncPixelTransferCompletionObserver* observer) = 0;
 
   virtual uint32 GetTextureUploadCount() = 0;
   virtual base::TimeDelta GetTotalTextureUploadTime() = 0;
 
   // ProcessMorePendingTransfers() will be called at a good time
   // to process a small amount of pending transfer work while
@@ skipping 35 matching lines @@
   virtual AsyncPixelTransferDelegate* CreatePixelTransferDelegateImpl(
       gles2::TextureRef* ref,
       const AsyncTexImage2DParams& define_params) = 0;
 
   DISALLOW_COPY_AND_ASSIGN(AsyncPixelTransferManager);
 };
 
 }  // namespace gpu
 
 #endif  // GPU_COMMAND_BUFFER_SERVICE_ASYNC_PIXEL_TRANSFER_MANAGER_H_