Re-land: Mark async texture uploads as completed from the upload thread.

ui/gl/async_pixel_transfer_delegate_android.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 "ui/gl/async_pixel_transfer_delegate_android.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/process_util.h"
 #include "base/shared_memory.h"
 #include "base/threading/thread.h"
 #include "build/build_config.h"
+#include "gpu/command_buffer/common/gles2_cmd_format.h"
 #include "ui/gl/async_pixel_transfer_delegate.h"
 #include "ui/gl/async_pixel_transfer_delegate_stub.h"
 #include "ui/gl/egl_util.h"
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_surface_egl.h"
 
 // TODO(epenner): Move thread priorities to base. (crbug.com/170549)
 #include <sys/resource.h>
 
 using base::SharedMemory;
 using base::SharedMemoryHandle;
 
 namespace gfx {
 
 namespace {
 
+class TextureUploadStats

[epenner, 2013/02/07 20:36:34]

Can this be another change? I don't understand how upload stats are related to
this change. And it adds another lock, each instance of which requires a lot of
justification, IMO.

[reveman, 2013/02/07 21:30:52]

I can't get rid of the replies and keep collecting upload stats correctly
without this. we can of course remove the stats but that breaks the benchmarks
extension.

[epenner, 2013/02/07 21:59:47]

I see, let's not get rid of the reply then? We should minimize the impact of
this latency savings optimization on design cleanliness.

Could the query code also benefit from the reply to reduce a lot of the changes
there? Could we do *only* the query completion on the upload thread and handle
the rest in a reply to the GPU thread?  It appears a lot of GPU side code was
written to work around not having the reply task.

+    : public base::RefCountedThreadSafe<TextureUploadStats> {
+ public:
+  TextureUploadStats() : texture_upload_count_(0) {}
+
+  void AddUpload(base::TimeDelta transfer_time) {
+    base::AutoLock scoped_lock(lock_);
+    texture_upload_count_++;
+    total_texture_upload_time_ += transfer_time;
+  }
+
+  int GetStats(base::TimeDelta* total_texture_upload_time) {
+    base::AutoLock scoped_lock(lock_);
+    if (total_texture_upload_time)
+      *total_texture_upload_time = total_texture_upload_time_;
+    return texture_upload_count_;
+  }
+
+ private:
+  friend class RefCountedThreadSafe<TextureUploadStats>;
+
+  ~TextureUploadStats() {}
+
+  int texture_upload_count_;
+  base::TimeDelta total_texture_upload_time_;
+  base::Lock lock_;
+};
+
 const char kAsyncTransferThreadName[] = "AsyncTransferThread";
 
 bool CheckErrors(const char* file, int line) {
   EGLint eglerror;
   GLenum glerror;
   bool success = true;
   while ((eglerror = eglGetError()) != EGL_SUCCESS) {
      LOG(ERROR) << "Async transfer EGL error at "
                 << file << ":" << line << " " << eglerror;
      success = false;
@@ skipping 100 matching lines @@
         transfer_in_progress_(false),
         egl_image_(EGL_NO_IMAGE_KHR),
         wait_for_uploads_(wait_for_uploads),
         wait_for_egl_images_(wait_for_egl_images) {
     static const AsyncTexImage2DParams zero_params = {0, 0, 0, 0, 0, 0, 0, 0};
     late_bind_define_params_ = zero_params;
   }
 
   // Implement AsyncPixelTransferState:
   bool TransferIsInProgress() {
+    base::AutoLock scoped_lock(transfer_in_progress_lock_);
     return transfer_in_progress_;
   }
 
   void BindTransfer(AsyncTexImage2DParams* bound_params) {
     TRACE_EVENT2("gpu", "BindAsyncTransfer glEGLImageTargetTexture2DOES",
                  "width", late_bind_define_params_.width,
                  "height", late_bind_define_params_.height);
 
     DCHECK(bound_params);
     DCHECK(texture_id_);
@@ skipping 61 matching lines @@
     // This glFinish is just a safe-guard for if uploads have some
     // GPU action that needs to occur. We could use fences and try
     // to do this less often. However, on older drivers fences are
     // not always reliable (eg. Mali-400 just blocks forever).
     if (wait_for_uploads_) {
       TRACE_EVENT0("gpu", "glFinish");
       glFinish();
     }
   }
 
+  void MarkAsCompleted() {
+    base::AutoLock scoped_lock(transfer_in_progress_lock_);
+    DCHECK(transfer_in_progress_);
+    transfer_in_progress_ = false;
+  }
+
  protected:
   friend class base::RefCountedThreadSafe<TransferStateInternal>;
   friend class AsyncPixelTransferDelegateAndroid;
 
   static void DeleteTexture(GLuint id) {
     glDeleteTextures(1, &id);
   }
 
   virtual ~TransferStateInternal() {
     if (egl_image_ != EGL_NO_IMAGE_KHR) {
@@ skipping 14 matching lines @@
 
   // Indicates there is a new EGLImage and the 'real'
   // texture needs to be bound to it as an EGLImage target.
   bool needs_late_bind_;
 
   // Definition params for texture that needs binding.
   AsyncTexImage2DParams late_bind_define_params_;
 
   // Indicates that an async transfer is in progress.
   bool transfer_in_progress_;
+  base::Lock transfer_in_progress_lock_;

[reveman, 2013/02/07 19:59:02]

we could use base::subtle::Atomic32 here instead if performance is a concern.

[epenner, 2013/02/07 20:22:56]

Can you explain a bit why we need a lock or atomic int at all?

[reveman, 2013/02/07 21:30:52]

Both threads will access this. Main thread to determine when transfer is done.
Upload thread sets it when done.

[epenner, 2013/02/07 21:59:47]

That's what atomic ints are used for yes, but I'm asking why it's required from
a design perspective. Could this benefit from the reply as well?

If the reply is posted to the GPU before we mark the completion, it's impossible
for the renderer to do something on the GPU thread before the GPU handles the
reply.

 
   // It would be nice if we could just create a new EGLImage for
   // 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_;
-
   // Customize when we block on fences (these are work-arounds).
   bool wait_for_uploads_;
   bool wait_for_egl_images_;
 };
 
 // Android needs thread-safe ref-counting, so this just wraps
 // an internal thread-safe ref-counted state object.
 class AsyncTransferStateAndroid : public AsyncPixelTransferState {
  public:
   explicit AsyncTransferStateAndroid(GLuint texture_id,
                                      bool wait_for_uploads,
                                      bool wait_for_egl_images)
       : internal_(new TransferStateInternal(texture_id,
                                             wait_for_uploads,
                                             wait_for_egl_images)) {
   }
   virtual ~AsyncTransferStateAndroid() {}
   virtual bool TransferIsInProgress() {
       return internal_->TransferIsInProgress();
   }
   virtual void BindTransfer(AsyncTexImage2DParams* bound_params) {
       internal_->BindTransfer(bound_params);
   }
   scoped_refptr<TransferStateInternal> internal_;
 };
 
 // Class which handles async pixel transfers on Android (using
 // EGLImageKHR and another upload thread)
-class AsyncPixelTransferDelegateAndroid
-    : public AsyncPixelTransferDelegate,
-      public base::SupportsWeakPtr<AsyncPixelTransferDelegateAndroid> {
+class AsyncPixelTransferDelegateAndroid : public AsyncPixelTransferDelegate {
  public:
   AsyncPixelTransferDelegateAndroid();
   virtual ~AsyncPixelTransferDelegateAndroid();
 
   // implement AsyncPixelTransferDelegate:
   virtual void AsyncNotifyCompletion(
-      const base::Closure& task) OVERRIDE;
+      const AsyncMemoryParams& mem_params,
+      uint32 submit_count) OVERRIDE;
   virtual void AsyncTexImage2D(
       AsyncPixelTransferState* state,
       const AsyncTexImage2DParams& tex_params,
       const AsyncMemoryParams& mem_params) OVERRIDE;
   virtual void AsyncTexSubImage2D(
       AsyncPixelTransferState* state,
       const AsyncTexSubImage2DParams& tex_params,
       const AsyncMemoryParams& mem_params) OVERRIDE;
   virtual uint32 GetTextureUploadCount() OVERRIDE;
   virtual base::TimeDelta GetTotalTextureUploadTime() OVERRIDE;
 
  private:
   // implement AsyncPixelTransferDelegate:
   virtual AsyncPixelTransferState*
       CreateRawPixelTransferState(GLuint texture_id) OVERRIDE;
 
-  void AsyncTexImage2DCompleted(scoped_refptr<TransferStateInternal> state);
-  void AsyncTexSubImage2DCompleted(scoped_refptr<TransferStateInternal> state);
-
   static void PerformAsyncTexImage2D(
       TransferStateInternal* state,
       AsyncTexImage2DParams tex_params,
       base::SharedMemory* shared_memory,
       uint32 shared_memory_data_offset);
   static void PerformAsyncTexSubImage2D(
       TransferStateInternal* state,
       AsyncTexSubImage2DParams tex_params,
       base::SharedMemory* shared_memory,
-      uint32 shared_memory_data_offset);
+      uint32 shared_memory_data_offset,
+      scoped_refptr<TextureUploadStats> texture_upload_stats);
+  static void PerformNotifyCompletion(
+      base::SharedMemory* shared_memory,
+      uint32 shared_memory_data_offset,
+      uint32 submit_count);
 
   // Returns true if a work-around was used.
   bool WorkAroundAsyncTexImage2D(
       TransferStateInternal* state,
       const AsyncTexImage2DParams& tex_params,
       const AsyncMemoryParams& mem_params);
   bool WorkAroundAsyncTexSubImage2D(
       TransferStateInternal* state,
       const AsyncTexSubImage2DParams& tex_params,
       const AsyncMemoryParams& mem_params);
 
-  int texture_upload_count_;
-  base::TimeDelta total_texture_upload_time_;
+  scoped_refptr<TextureUploadStats> texture_upload_stats_;
   bool is_imagination_;
   bool is_qualcomm_;
 
   DISALLOW_COPY_AND_ASSIGN(AsyncPixelTransferDelegateAndroid);
 };
 
 namespace {
 // Imagination has some odd problems still.
 bool IsImagination() {
   std::string vendor;
@@ skipping 19 matching lines @@
         static_cast<AsyncPixelTransferDelegate*>(
             new AsyncPixelTransferDelegateAndroid()));
   } else {
     LOG(INFO) << "Async pixel transfers not supported";
     return make_scoped_ptr(
         static_cast<AsyncPixelTransferDelegate*>(
             new AsyncPixelTransferDelegateStub()));
   }
 }
 
-AsyncPixelTransferDelegateAndroid::AsyncPixelTransferDelegateAndroid()
-    : texture_upload_count_(0) {
+AsyncPixelTransferDelegateAndroid::AsyncPixelTransferDelegateAndroid() {
   std::string vendor;
   vendor = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
   is_imagination_ = vendor.find("Imagination") != std::string::npos;
   is_qualcomm_ = vendor.find("Qualcomm") != std::string::npos;
+  // TODO(reveman): Skip this if --enable-gpu-benchmarking is not present.
+  texture_upload_stats_ = make_scoped_refptr(new TextureUploadStats);
 }
 
 AsyncPixelTransferDelegateAndroid::~AsyncPixelTransferDelegateAndroid() {
 }
 
 AsyncPixelTransferState*
     AsyncPixelTransferDelegateAndroid::
         CreateRawPixelTransferState(GLuint texture_id) {
 
   // We can't wait on uploads on imagination (it can take 200ms+).
   bool wait_for_uploads = !is_imagination_;
 
   // We need to wait for EGLImage creation on Qualcomm.
   bool wait_for_egl_images = is_qualcomm_;
 
   return static_cast<AsyncPixelTransferState*>(
       new AsyncTransferStateAndroid(texture_id,
                                     wait_for_uploads,
                                     wait_for_egl_images));
 }
 
-namespace {
-// Dummy function to measure completion on
-// the upload thread.
-void NoOp() {}
-} // namespace
-
 void AsyncPixelTransferDelegateAndroid::AsyncNotifyCompletion(
-      const base::Closure& task) {
-  // Post a no-op task to the upload thread followed
-  // by a reply to the callback. The reply will then occur after
-  // all async transfers are complete.
-  transfer_message_loop_proxy()->PostTaskAndReply(FROM_HERE,
-      base::Bind(&NoOp), task);
+    const AsyncMemoryParams& mem_params,
+    uint32 submit_count) {
+  DCHECK(mem_params.shared_memory);
+  DCHECK_LE(mem_params.shm_data_offset + mem_params.shm_data_size,
+            mem_params.shm_size);
+  // Post a PerformNotifyCompletion task to the upload thread. This task
+  // will run after all async transfers are complete and mark the QuerySync as
+  // completed.
+  transfer_message_loop_proxy()->PostTask(
+      FROM_HERE,
+      base::Bind(&AsyncPixelTransferDelegateAndroid::PerformNotifyCompletion,
+                 base::Owned(DuplicateSharedMemory(mem_params.shared_memory,
+                                                   mem_params.shm_size)),
+                 mem_params.shm_data_offset,
+                 submit_count));
 }
 
 void AsyncPixelTransferDelegateAndroid::AsyncTexImage2D(
     AsyncPixelTransferState* transfer_state,
     const AsyncTexImage2DParams& tex_params,
     const AsyncMemoryParams& mem_params) {
   scoped_refptr<TransferStateInternal> state =
       static_cast<AsyncTransferStateAndroid*>(transfer_state)->internal_.get();
   DCHECK(mem_params.shared_memory);
   DCHECK_LE(mem_params.shm_data_offset + mem_params.shm_data_size,
             mem_params.shm_size);
   DCHECK(state);
   DCHECK(state->texture_id_);
   DCHECK(!state->needs_late_bind_);
   DCHECK(!state->transfer_in_progress_);
   DCHECK_EQ(state->egl_image_, EGL_NO_IMAGE_KHR);
   DCHECK_EQ(static_cast<GLenum>(GL_TEXTURE_2D), tex_params.target);
   DCHECK_EQ(tex_params.level, 0);
 
   if (WorkAroundAsyncTexImage2D(state, tex_params, mem_params))
     return;
 
   // Mark the transfer in progress and save define params for lazy binding.
+  state->needs_late_bind_ = true;
   state->transfer_in_progress_ = true;
   state->late_bind_define_params_ = tex_params;
 
   // Duplicate the shared memory so there are no way we can get
   // a use-after-free of the raw pixels.
-  transfer_message_loop_proxy()->PostTaskAndReply(FROM_HERE,
-      base::Bind(
-          &AsyncPixelTransferDelegateAndroid::PerformAsyncTexImage2D,
-          base::Unretained(state.get()),  // This is referenced in reply below.
-          tex_params,
-          base::Owned(DuplicateSharedMemory(mem_params.shared_memory,
-                                            mem_params.shm_size)),
-          mem_params.shm_data_offset),
-      base::Bind(
-          &AsyncPixelTransferDelegateAndroid::AsyncTexImage2DCompleted,
-          AsWeakPtr(),
-          state));
+  transfer_message_loop_proxy()->PostTask(FROM_HERE,
+      base::Bind(&AsyncPixelTransferDelegateAndroid::PerformAsyncTexImage2D,
+                 state,
+                 tex_params,
+                 base::Owned(DuplicateSharedMemory(mem_params.shared_memory,
+                                                   mem_params.shm_size)),
+                 mem_params.shm_data_offset));
 
   DCHECK(CHECK_GL());
 }
 
 void AsyncPixelTransferDelegateAndroid::AsyncTexSubImage2D(
     AsyncPixelTransferState* transfer_state,
     const AsyncTexSubImage2DParams& tex_params,
     const AsyncMemoryParams& mem_params) {
   TRACE_EVENT2("gpu", "AsyncTexSubImage2D",
                "width", tex_params.width,
@@ skipping 13 matching lines @@
 
   // Mark the transfer in progress.
   state->transfer_in_progress_ = true;
 
   // If this wasn't async allocated, we don't have an EGLImage yet.
   // Create the EGLImage if it hasn't already been created.
   state->CreateEglImageOnMainThreadIfNeeded();
 
   // Duplicate the shared memory so there are no way we can get
   // a use-after-free of the raw pixels.
-  transfer_message_loop_proxy()->PostTaskAndReply(FROM_HERE,
-      base::Bind(
-          &AsyncPixelTransferDelegateAndroid::PerformAsyncTexSubImage2D,
-          base::Unretained(state.get()),  // This is referenced in reply below.
-          tex_params,
-          base::Owned(DuplicateSharedMemory(mem_params.shared_memory,
-                                            mem_params.shm_size)),
-          mem_params.shm_data_offset),
-      base::Bind(
-          &AsyncPixelTransferDelegateAndroid::AsyncTexSubImage2DCompleted,
-          AsWeakPtr(),
-          state));
+  transfer_message_loop_proxy()->PostTask(FROM_HERE,
+      base::Bind(&AsyncPixelTransferDelegateAndroid::PerformAsyncTexSubImage2D,
+                 state,
+                 tex_params,
+                 base::Owned(DuplicateSharedMemory(mem_params.shared_memory,
+                                                   mem_params.shm_size)),
+                 mem_params.shm_data_offset,
+                 texture_upload_stats_));
 
   DCHECK(CHECK_GL());
 }
 
 uint32 AsyncPixelTransferDelegateAndroid::GetTextureUploadCount() {
-  return texture_upload_count_;
+  CHECK(texture_upload_stats_);
+  return texture_upload_stats_->GetStats(NULL);
 }
 
 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_;
+  CHECK(texture_upload_stats_);
+  base::TimeDelta total_texture_upload_time;
+  texture_upload_stats_->GetStats(&total_texture_upload_time);
+  return total_texture_upload_time;
 }
 
 namespace {
 void SetGlParametersForEglImageTexture() {
   // These params are needed for EGLImage creation to succeed on several
   // Android devices. I couldn't find this requirement in the EGLImage
   // extension spec, but several devices fail without it.
   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
@@ skipping 47 matching lines @@
         0,
         0,
         tex_params.width,
         tex_params.height,
         tex_params.format,
         tex_params.type,
         data);
   }
 
   state->WaitForLastUpload();
+  state->MarkAsCompleted();
+
   DCHECK(CHECK_GL());
 }
 
 void AsyncPixelTransferDelegateAndroid::PerformAsyncTexSubImage2D(
     TransferStateInternal* state,
     AsyncTexSubImage2DParams tex_params,
     base::SharedMemory* shared_memory,
-    uint32 shared_memory_data_offset) {
+    uint32 shared_memory_data_offset,
+    scoped_refptr<TextureUploadStats> texture_upload_stats) {
   TRACE_EVENT2("gpu", "PerformAsyncTexSubImage2D",
                "width", tex_params.width,
                "height", tex_params.height);
 
   DCHECK(state);
   DCHECK_NE(EGL_NO_IMAGE_KHR, state->egl_image_);
   DCHECK_EQ(0, tex_params.level);
 
   state->WaitOnEglImageCreation();
 
   void* data = GetAddress(shared_memory, shared_memory_data_offset);
 
-  base::TimeTicks begin_time(base::TimeTicks::HighResNow());
+  base::TimeTicks begin_time;
+  if (texture_upload_stats)
+    begin_time = base::TimeTicks::HighResNow();
+
   if (!state->thread_texture_id_) {
     TRACE_EVENT0("gpu", "glEGLImageTargetTexture2DOES");
     glGenTextures(1, &state->thread_texture_id_);
     glActiveTexture(GL_TEXTURE0);
     glBindTexture(GL_TEXTURE_2D, state->thread_texture_id_);
     glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, state->egl_image_);
   } else {
     glActiveTexture(GL_TEXTURE0);
     glBindTexture(GL_TEXTURE_2D, state->thread_texture_id_);
   }
   {
     TRACE_EVENT0("gpu", "glTexSubImage2D");
     glTexSubImage2D(
         GL_TEXTURE_2D,
         tex_params.level,
         tex_params.xoffset,
         tex_params.yoffset,
         tex_params.width,
         tex_params.height,
         tex_params.format,
         tex_params.type,
         data);
   }
   state->WaitForLastUpload();
+  state->MarkAsCompleted();
 
   DCHECK(CHECK_GL());
-  state->last_transfer_time_ = base::TimeTicks::HighResNow() - begin_time;
+
+  if (texture_upload_stats) {
+    texture_upload_stats->AddUpload(
+        base::TimeTicks::HighResNow() - begin_time);
+  }
 }
 
+void AsyncPixelTransferDelegateAndroid::PerformNotifyCompletion(
+    base::SharedMemory* shared_memory,
+    uint32 shared_memory_data_offset,
+    uint32 submit_count) {
+  TRACE_EVENT0("gpu", "PerformNotifyCompletion");
+
+  gpu::gles2::QuerySync* sync = static_cast<gpu::gles2::QuerySync*>(
+      GetAddress(shared_memory, shared_memory_data_offset));
+  if (!sync)
+    return;
+  sync->process_count = submit_count;
+}
 
 namespace {
 bool IsPowerOfTwo (unsigned int x) {
   return ((x != 0) && !(x & (x - 1)));
 }
 
 bool IsMultipleOfEight(unsigned int x) {
   return (x & 7) == 0;
 }
 
@@ skipping 74 matching lines @@
     return false;
 
   // If the dimensions support fast async uploads, we can use the
   // normal async upload path for uploads.
   if (DimensionsSupportImgFastPath(tex_params.width, tex_params.height))
     return false;
 
   // Fall back on a synchronous stub as we don't have a known fast path.
   void* data = GetAddress(mem_params.shared_memory,
                           mem_params.shm_data_offset);
-  base::TimeTicks begin_time(base::TimeTicks::HighResNow());
+  base::TimeTicks begin_time;
+  if (texture_upload_stats_)
+    begin_time = base::TimeTicks::HighResNow();
   {
     TRACE_EVENT0("gpu", "glTexSubImage2D");
     glTexSubImage2D(
         tex_params.target,
         tex_params.level,
         tex_params.xoffset,
         tex_params.yoffset,
         tex_params.width,
         tex_params.height,
         tex_params.format,
         tex_params.type,
         data);
   }
-  texture_upload_count_++;
-  total_texture_upload_time_ += base::TimeTicks::HighResNow() - begin_time;
+  if (texture_upload_stats_) {
+    texture_upload_stats_->AddUpload(
+        base::TimeTicks::HighResNow() - begin_time);
+  }
 
   DCHECK(CHECK_GL());
   return true;
 }
 
 }  // namespace gfx