gpu: Report time spent performing async texture uploads.

ui/gl/async_pixel_transfer_delegate_android.cc

Index: ui/gl/async_pixel_transfer_delegate_android.cc
diff --git a/ui/gl/async_pixel_transfer_delegate_android.cc b/ui/gl/async_pixel_transfer_delegate_android.cc
index dd398ad29d3bfa8890a6dbdf167c7892f9417b21..3e7937f5c3017b99a8af214090b8342a7a58f2b6 100644
--- a/ui/gl/async_pixel_transfer_delegate_android.cc
+++ b/ui/gl/async_pixel_transfer_delegate_android.cc
@@ -154,16 +154,7 @@ class TransferStateInternal
     needs_late_bind_ = false;
   }
 
-  // Completion callbacks.
-  void TexImage2DCompleted() {
-    needs_late_bind_ = true;
-    transfer_in_progress_ = false;
-  }
-  void TexSubImage2DCompleted() {
-    transfer_in_progress_ = false;
-  }
-
-protected:
+ protected:
   friend class base::RefCountedThreadSafe<TransferStateInternal>;
   friend class AsyncPixelTransferDelegateAndroid;
 
@@ -191,6 +182,9 @@ protected:
   // every upload, but I found that didn't work, so this stores
   // one for the lifetime of the texture.
   EGLImageKHR egl_image_;
+
+  // Time spent performing last transfer.
+  base::TimeDelta last_transfer_time_;
 };
 
 // Android needs thread-safe ref-counting, so this just wraps
@@ -214,8 +208,8 @@ class AsyncTransferStateAndroid : public AsyncPixelTransferState {
 // EGLImageKHR and another upload thread)
 class AsyncPixelTransferDelegateAndroid : public AsyncPixelTransferDelegate {
  public:
-  AsyncPixelTransferDelegateAndroid() {}
-  virtual ~AsyncPixelTransferDelegateAndroid() {}
+  AsyncPixelTransferDelegateAndroid();
+  virtual ~AsyncPixelTransferDelegateAndroid();
 
   // implement AsyncPixelTransferDelegate:
   virtual void AsyncNotifyCompletion(
@@ -228,6 +222,8 @@ class AsyncPixelTransferDelegateAndroid : public AsyncPixelTransferDelegate {
       AsyncPixelTransferState* state,
       const AsyncTexSubImage2DParams& tex_params,
       const AsyncMemoryParams& mem_params) OVERRIDE;
+  virtual uint32 GetTextureUploadCount() OVERRIDE;
+  virtual base::TimeDelta GetTotalTextureUploadTime() OVERRIDE;
 
  private:
   // implement AsyncPixelTransferDelegate:
@@ -237,14 +233,27 @@ class AsyncPixelTransferDelegateAndroid : public AsyncPixelTransferDelegate {
   base::MessageLoopProxy* transfer_message_loop_proxy() {
     return g_transfer_thread.Pointer()->message_loop_proxy();
   }
+
+  void AsyncTexImage2DCompleted(scoped_refptr<TransferStateInternal> state);
+  void AsyncTexSubImage2DCompleted(scoped_refptr<TransferStateInternal> state);
+
   static void PerformAsyncTexImage2D(
       TransferStateInternal* state,
       AsyncTexImage2DParams tex_params,
-      AsyncMemoryParams mem_params);
+      base::SharedMemory* shared_memory,

[epenner, 2012/12/22 03:22:42]

Is there no way to keep this as one parameter?  Not sure it's worth the other
cleanup..

[epenner, 2012/12/22 20:48:17]

Hmm, could we just pass a "Safe" shared memory parameter in addition to the old
struct?  Then it will be easy to test the unsafe version (I still want to test
that it has no performance impact)

[reveman, 2012/12/22 21:56:24]

I guess we could do that. I removed shared_memory_size and
shared_memory_data_size parameters from the latest patch as it seemed
unnecessary to pass those to the other thread. I'll switch to a "safe" shm mem
parameter if you prefer that.

+      uint32 shared_memory_size,
+      uint32 shared_memory_data_offset,
+      uint32 shared_memory_data_size);
   static void PerformAsyncTexSubImage2D(
       TransferStateInternal* state,
       AsyncTexSubImage2DParams tex_params,
-      AsyncMemoryParams mem_params);
+      base::SharedMemory* shared_memory,
+      uint32 shared_memory_size,
+      uint32 shared_memory_data_offset,
+      uint32 shared_memory_data_size);
+
+  int texture_upload_count_;
+  base::TimeDelta total_texture_upload_time_;
 
   DISALLOW_COPY_AND_ASSIGN(AsyncPixelTransferDelegateAndroid);
 };
@@ -276,6 +285,13 @@ scoped_ptr<AsyncPixelTransferDelegate>
   }
 }
 
+AsyncPixelTransferDelegateAndroid::AsyncPixelTransferDelegateAndroid()
+    : texture_upload_count_(0) {
+}
+
+AsyncPixelTransferDelegateAndroid::~AsyncPixelTransferDelegateAndroid() {
+}
+
 AsyncPixelTransferState*
     AsyncPixelTransferDelegateAndroid::
         CreateRawPixelTransferState(GLuint texture_id) {
@@ -302,7 +318,7 @@ void AsyncPixelTransferDelegateAndroid::AsyncTexImage2D(
     AsyncPixelTransferState* transfer_state,
     const AsyncTexImage2DParams& tex_params,
     const AsyncMemoryParams& mem_params) {
-  TransferStateInternal* state =
+  scoped_refptr<TransferStateInternal> state =
       static_cast<AsyncTransferStateAndroid*>(transfer_state)->internal_.get();
   DCHECK(mem_params.shared_memory);
   DCHECK(state);
@@ -319,18 +335,19 @@ void AsyncPixelTransferDelegateAndroid::AsyncTexImage2D(
 
   // Duplicate the shared memory so there are no way we can get
   // a use-after-free of the raw pixels.
-  // TODO: Could we pass an own pointer of the new SharedMemory to the task?
-  AsyncMemoryParams duped_mem = mem_params;
-  duped_mem.shared_memory = DuplicateSharedMemory(mem_params.shared_memory,
-                                                  mem_params.shm_size);
   transfer_message_loop_proxy()->PostTaskAndReply(FROM_HERE,
       base::Bind(
           &AsyncPixelTransferDelegateAndroid::PerformAsyncTexImage2D,
-          base::Unretained(state), // This is referenced in reply below.
+          base::Unretained(state.get()),  // This is referenced in reply below.
           tex_params,
-          duped_mem),
+          base::Owned(DuplicateSharedMemory(mem_params.shared_memory,
+                                            mem_params.shm_size)),
+          mem_params.shm_size,
+          mem_params.shm_data_offset,
+          mem_params.shm_data_size),
       base::Bind(
-          &TransferStateInternal::TexImage2DCompleted,
+          &AsyncPixelTransferDelegateAndroid::AsyncTexImage2DCompleted,
+          base::Unretained(this),

[epenner, 2012/12/22 03:22:42]

I don't think it's safe to pass this pointer as is. Uploads can out-live the
transfer-delegate.  The state is kept alive.

Hmm, it looks like the delegate either needs to support weak pointer, or the
uploads stats could be stored in the thread object.

[reveman, 2012/12/22 21:56:24]

added support for weak pointer in latest patch. it would be nice to make the
transfer thread part of the transfer delegate so it will be shut down and all
pending tasks are completed when destroying the delegate. Looks like
TransferStateInternal::thread_texture_id would have to be registered with the
transfer thread so it could be deleted at thread cleanup.

           state));
 }
 
@@ -341,7 +358,7 @@ void AsyncPixelTransferDelegateAndroid::AsyncTexSubImage2D(
   TRACE_EVENT2("gpu", "AsyncTexSubImage2D",
                "width", tex_params.width,
                "height", tex_params.height);
-  TransferStateInternal* state =
+  scoped_refptr<TransferStateInternal> state =
       static_cast<AsyncTransferStateAndroid*>(transfer_state)->internal_.get();
   DCHECK(state->texture_id_);
   DCHECK(!state->transfer_in_progress_);
@@ -374,21 +391,43 @@ void AsyncPixelTransferDelegateAndroid::AsyncTexSubImage2D(
 
   // Duplicate the shared memory so there are no way we can get
   // a use-after-free of the raw pixels.
-  // TODO: Could we pass an own pointer of the new SharedMemory to the task?
-  AsyncMemoryParams duped_mem = mem_params;
-  duped_mem.shared_memory = DuplicateSharedMemory(mem_params.shared_memory,
-                                                  mem_params.shm_size);
   transfer_message_loop_proxy()->PostTaskAndReply(FROM_HERE,
       base::Bind(
           &AsyncPixelTransferDelegateAndroid::PerformAsyncTexSubImage2D,
-          base::Unretained(state), // This is referenced in reply below.
+          base::Unretained(state.get()),  // This is referenced in reply below.
           tex_params,
-          duped_mem),
+          base::Owned(DuplicateSharedMemory(mem_params.shared_memory,
+                                            mem_params.shm_size)),
+          mem_params.shm_size,
+          mem_params.shm_data_offset,
+          mem_params.shm_data_size),
       base::Bind(
-          &TransferStateInternal::TexSubImage2DCompleted,
+          &AsyncPixelTransferDelegateAndroid::AsyncTexSubImage2DCompleted,
+          base::Unretained(this),
           state));
 }
 
+uint32 AsyncPixelTransferDelegateAndroid::GetTextureUploadCount() {
+  return texture_upload_count_;
+}
+
+base::TimeDelta AsyncPixelTransferDelegateAndroid::GetTotalTextureUploadTime() {
+  return total_texture_upload_time_;
+}
+
+void AsyncPixelTransferDelegateAndroid::AsyncTexImage2DCompleted(
+    scoped_refptr<TransferStateInternal> state) {
+  state->needs_late_bind_ = true;
+  state->transfer_in_progress_ = false;
+}
+
+void AsyncPixelTransferDelegateAndroid::AsyncTexSubImage2DCompleted(
+    scoped_refptr<TransferStateInternal> state) {
+  state->transfer_in_progress_ = false;
+  texture_upload_count_++;
+  total_texture_upload_time_ += state->last_transfer_time_;
+}
+
 namespace {
 void WaitForGlFence() {
   // Uploads usually finish on the CPU, but just in case add a fence
@@ -411,15 +450,14 @@ void WaitForGlFence() {
 void AsyncPixelTransferDelegateAndroid::PerformAsyncTexImage2D(
     TransferStateInternal* state,
     AsyncTexImage2DParams tex_params,
-    AsyncMemoryParams mem_params) {
-  // TODO(epenner): This is just to insure it is deleted. Could bind() do this?
-  scoped_ptr<SharedMemory> shared_memory =
-      make_scoped_ptr(mem_params.shared_memory);
-
-  void* data = GetAddress(mem_params.shared_memory,
-                          mem_params.shm_size,
-                          mem_params.shm_data_offset,
-                          mem_params.shm_data_size);
+    base::SharedMemory* shared_memory,
+    uint32 shared_memory_size,
+    uint32 shared_memory_data_offset,
+    uint32 shared_memory_data_size) {
+  void* data = GetAddress(shared_memory,
+                          shared_memory_size,
+                          shared_memory_data_offset,
+                          shared_memory_data_size);
 
   // In texImage2D, we do everything on the upload thread. This is
   // because texImage2D can incur CPU allocation cost, and it also
@@ -475,15 +513,14 @@ void AsyncPixelTransferDelegateAndroid::PerformAsyncTexImage2D(
 void AsyncPixelTransferDelegateAndroid::PerformAsyncTexSubImage2D(
     TransferStateInternal* state,
     AsyncTexSubImage2DParams tex_params,
-    AsyncMemoryParams mem_params) {
-  // TODO(epenner): This is just to insure it is deleted. Could bind() do this?
-  scoped_ptr<SharedMemory> shared_memory =
-      make_scoped_ptr(mem_params.shared_memory);
-
-  void* data = GetAddress(mem_params.shared_memory,
-                          mem_params.shm_size,
-                          mem_params.shm_data_offset,
-                          mem_params.shm_data_size);
+    base::SharedMemory* shared_memory,
+    uint32 shared_memory_size,
+    uint32 shared_memory_data_offset,
+    uint32 shared_memory_data_size) {
+  void* data = GetAddress(shared_memory,
+                          shared_memory_size,
+                          shared_memory_data_offset,
+                          shared_memory_data_size);
 
   // For a texSubImage, the texture must already have been
   // created on the main thread, along with EGLImageKHR.
@@ -493,6 +530,7 @@ void AsyncPixelTransferDelegateAndroid::PerformAsyncTexSubImage2D(
                "width", tex_params.width,
                "height", tex_params.height);
 
+  base::TimeTicks begin_time(base::TimeTicks::HighResNow());
   // Create a texture from the image and upload to it.
   GLuint temp_texture = 0;
   glGenTextures(1, &temp_texture);
@@ -517,6 +555,7 @@ void AsyncPixelTransferDelegateAndroid::PerformAsyncTexSubImage2D(
   // We can delete this thread's texture as the real texture
   // now contains the data.
   glDeleteTextures(1, &temp_texture);
+  state->last_transfer_time_ = base::TimeTicks::HighResNow() - begin_time;
 }
 
 }  // namespace gfx