Share SyncPointManager implementation in in-process cmd buffer

gpu/command_buffer/service/in_process_command_buffer.cc

 // 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.
 
 #include "gpu/command_buffer/service/in_process_command_buffer.h"
 
 #include <queue>
 #include <set>
 #include <utility>
 
@@ skipping 13 matching lines @@
 #include "gpu/command_buffer/service/context_group.h"
 #include "gpu/command_buffer/service/gl_context_virtual.h"
 #include "gpu/command_buffer/service/gpu_scheduler.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "gpu/command_buffer/service/image_factory.h"
 #include "gpu/command_buffer/service/image_manager.h"
 #include "gpu/command_buffer/service/mailbox_manager_impl.h"
 #include "gpu/command_buffer/service/mailbox_manager_sync.h"
 #include "gpu/command_buffer/service/memory_tracking.h"
 #include "gpu/command_buffer/service/query_manager.h"
+#include "gpu/command_buffer/service/sync_point_manager.h"
 #include "gpu/command_buffer/service/transfer_buffer_manager.h"
 #include "gpu/command_buffer/service/valuebuffer_manager.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_image.h"
 #include "ui/gl/gl_share_group.h"
 
 #if defined(OS_ANDROID)
 #include "gpu/command_buffer/service/stream_texture_manager_in_process_android.h"
 #include "ui/gl/android/surface_texture.h"
@@ skipping 79 matching lines @@
 
 class ScopedEvent {
  public:
   ScopedEvent(base::WaitableEvent* event) : event_(event) {}
   ~ScopedEvent() { event_->Signal(); }
 
  private:
   base::WaitableEvent* event_;
 };
 
-class SyncPointManager {
+class SyncPointManagerWrapper {
  public:
-  SyncPointManager();
-  ~SyncPointManager();
+  SyncPointManagerWrapper();
 
   uint32 GenerateSyncPoint();
   void RetireSyncPoint(uint32 sync_point);
+  void AddSyncPointCallback(uint32 sync_point, const base::Closure& callback);
 
-  bool IsSyncPointPassed(uint32 sync_point);
   void WaitSyncPoint(uint32 sync_point);
 
-private:
-  // This lock protects access to pending_sync_points_ and next_sync_point_ and
-  // is used with the ConditionVariable to signal when a sync point is retired.
-  base::Lock lock_;
-  std::set<uint32> pending_sync_points_;
-  uint32 next_sync_point_;
-  base::ConditionVariable cond_var_;
+ private:
+  void SyncPointRetired();
+
+  const scoped_refptr<SyncPointManager> manager_;
+  base::Lock retire_lock_;
+  base::ConditionVariable retire_cond_var_;
+
+  DISALLOW_COPY_AND_ASSIGN(SyncPointManagerWrapper);
 };
 
-SyncPointManager::SyncPointManager() : next_sync_point_(1), cond_var_(&lock_) {}
-
-SyncPointManager::~SyncPointManager() {
-  DCHECK_EQ(pending_sync_points_.size(), 0U);
+SyncPointManagerWrapper::SyncPointManagerWrapper()
+    : manager_(new SyncPointManager), retire_cond_var_(&retire_lock_) {
 }
 
-uint32 SyncPointManager::GenerateSyncPoint() {
-  base::AutoLock lock(lock_);
-  uint32 sync_point = next_sync_point_++;
-  DCHECK_EQ(pending_sync_points_.count(sync_point), 0U);
-  pending_sync_points_.insert(sync_point);
+uint32 SyncPointManagerWrapper::GenerateSyncPoint() {
+  uint32 sync_point = manager_->GenerateSyncPoint();
+  manager_->AddSyncPointCallback(
+      sync_point, base::Bind(&SyncPointManagerWrapper::SyncPointRetired,
+                             base::Unretained(this)));
   return sync_point;
 }
 
-void SyncPointManager::RetireSyncPoint(uint32 sync_point) {
-  base::AutoLock lock(lock_);
-  DCHECK(pending_sync_points_.count(sync_point));
-  pending_sync_points_.erase(sync_point);
-  cond_var_.Broadcast();
+void SyncPointManagerWrapper::RetireSyncPoint(uint32 sync_point) {
+  manager_->RetireSyncPoint(sync_point);
 }
 
-bool SyncPointManager::IsSyncPointPassed(uint32 sync_point) {
-  base::AutoLock lock(lock_);
-  return pending_sync_points_.count(sync_point) == 0;
+void SyncPointManagerWrapper::AddSyncPointCallback(
+    uint32 sync_point,
+    const base::Closure& callback) {
+  manager_->AddSyncPointCallback(sync_point, callback);
 }
 
-void SyncPointManager::WaitSyncPoint(uint32 sync_point) {
-  base::AutoLock lock(lock_);
-  while (pending_sync_points_.count(sync_point)) {
-    cond_var_.Wait();
+void SyncPointManagerWrapper::WaitSyncPoint(uint32 sync_point) {
+  base::AutoLock lock(retire_lock_);
+  while (!manager_->IsSyncPointRetired(sync_point)) {
+    retire_cond_var_.Wait();
   }
 }
 
-base::LazyInstance<SyncPointManager> g_sync_point_manager =
+void SyncPointManagerWrapper::SyncPointRetired() {
+  base::AutoLock lock(retire_lock_);
+  retire_cond_var_.Broadcast();
+}
+
+base::LazyInstance<SyncPointManagerWrapper> g_sync_point_manager =
     LAZY_INSTANCE_INITIALIZER;
 
 base::SharedMemoryHandle ShareToGpuThread(
     base::SharedMemoryHandle source_handle) {
 #if defined(OS_WIN)
   // Windows needs to explicitly duplicate the handle to current process.
   base::SharedMemoryHandle target_handle;
   if (!DuplicateHandle(GetCurrentProcess(),
                        source_handle,
                        GetCurrentProcess(),
@@ skipping 627 matching lines @@
       decoder_->GetContextGroup()->mailbox_manager();
   if (mailbox_manager->UsesSync()) {
     bool make_current_success = false;
     {
       base::AutoLock lock(command_buffer_lock_);
       make_current_success = MakeCurrent();
     }
     if (make_current_success)
       mailbox_manager->PushTextureUpdates(sync_point);
   }
+
   g_sync_point_manager.Get().RetireSyncPoint(sync_point);
 }
 
 void InProcessCommandBuffer::SignalSyncPoint(unsigned sync_point,
                                              const base::Closure& callback) {
   CheckSequencedThread();
   QueueTask(base::Bind(&InProcessCommandBuffer::SignalSyncPointOnGpuThread,
                        base::Unretained(this),
                        sync_point,
                        WrapCallback(callback)));
 }
 
 bool InProcessCommandBuffer::WaitSyncPointOnGpuThread(unsigned sync_point) {
   g_sync_point_manager.Get().WaitSyncPoint(sync_point);
+
   gles2::MailboxManager* mailbox_manager =
       decoder_->GetContextGroup()->mailbox_manager();
   mailbox_manager->PullTextureUpdates(sync_point);
   return true;
 }
 
 void InProcessCommandBuffer::SignalSyncPointOnGpuThread(
     unsigned sync_point,
     const base::Closure& callback) {
-  if (g_sync_point_manager.Get().IsSyncPointPassed(sync_point)) {
-    callback.Run();
-  } else {
-    service_->ScheduleIdleWork(
-        base::Bind(&InProcessCommandBuffer::SignalSyncPointOnGpuThread,
-                   gpu_thread_weak_ptr_,
-                   sync_point,
-                   callback));
-  }
+  g_sync_point_manager.Get().AddSyncPointCallback(sync_point, callback);

[boliu, 2015/01/28 00:49:03]

Question. The callback is no longer guaranteed to be called on this gpu thread
now. Is that a problem? Should we wrap it here to post it back to this gpu
thread?

[boliu, 2015/01/28 17:48:17]

I think it should be posted back to this gpu thread:

The callback is already wrapped to run on the client thread, with the caveat
that it doesn't work if the client thread is the render thread and doesn't have
a message loop.

Previously the wrapped callback is guaranteed to be called on this gpu thread,
so the caveat only appens, if client thread == gpu thread == render thread, and
everything was "fine".

But now the wrapped callback can be called on another gpu thread, and in that
edge case, the actual callback will be on the wrong thread.

[boliu, 2015/01/28 17:53:50]

Wait, can the sync point be retired on another gpu thread? That shouldn't be
allowed, right? In that case, this is be ok :p

 }
 
 void InProcessCommandBuffer::SignalQuery(unsigned query_id,
                                          const base::Closure& callback) {
   CheckSequencedThread();
   QueueTask(base::Bind(&InProcessCommandBuffer::SignalQueryOnGpuThread,
                        base::Unretained(this),
                        query_id,
                        WrapCallback(callback)));
 }
@@ skipping 81 matching lines @@
 
 #if defined(OS_ANDROID)
 scoped_refptr<gfx::SurfaceTexture>
 InProcessCommandBuffer::GetSurfaceTexture(uint32 stream_id) {
   DCHECK(stream_texture_manager_);
   return stream_texture_manager_->GetSurfaceTexture(stream_id);
 }
 #endif
 
 }  // namespace gpu