Free worker context resources on idle.

gpu/command_buffer/client/gles2_implementation.cc

Index: gpu/command_buffer/client/gles2_implementation.cc
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 1c5cd13e9936d3bf84bbdef6f53e225ee353cefb..6ea86583fbd62ffe2453c2216d1a5070998470ab 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -53,11 +53,13 @@ namespace gles2 {
 namespace {
 
 // Class that DCHECKs if it is destructed without first having Release called.
-class ScopedVisibilityImpl : public ContextSupport::ScopedVisibility {
+template <typename T>
+class ScopedContextSupportToken : public T {
  public:
-  explicit ScopedVisibilityImpl(ContextSupport* context_support)
+  explicit ScopedContextSupportToken(ContextSupport* context_support)
       : initial_context_support_(context_support) {}
-  ~ScopedVisibilityImpl() { DCHECK(!initial_context_support_); }
+
+  ~ScopedContextSupportToken() { DCHECK(!initial_context_support_); }
 
   void Release(ContextSupport* context_support) {
     DCHECK_EQ(initial_context_support_, context_support);
@@ -68,6 +70,10 @@ class ScopedVisibilityImpl : public ContextSupport::ScopedVisibility {
   const ContextSupport* initial_context_support_;
 };
 
+using ScopedVisibilityImpl =
+    ScopedContextSupportToken<ContextSupport::ScopedVisibility>;
+using ScopedBusyImpl = ScopedContextSupportToken<ContextSupport::ScopedBusy>;
+
 void CopyRectToBuffer(const void* pixels,
                       uint32_t height,
                       uint32_t unpadded_row_size,
@@ -101,6 +107,8 @@ uint32_t GenerateNextFlushId() {
   return static_cast<uint32_t>(g_flush_id.GetNext());
 }
 
+const base::TimeDelta kIdleCallbackDelay = base::TimeDelta::FromSeconds(1);
+
 }  // anonymous namespace
 
 #if !defined(_MSC_VER)
@@ -292,6 +300,8 @@ GLES2Implementation::~GLES2Implementation() {
   base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
       this);
 
+  CancelIdleCallback();
+
   // Make sure the queries are finished otherwise we'll delete the
   // shared memory (mapped_memory_) which will free the memory used
   // by the queries. The GPU process when validating that memory is still
@@ -446,6 +456,18 @@ void GLES2Implementation::SetAggressivelyFreeResources(
   }
 }
 
+void GLES2Implementation::TrimResources() {
+  TRACE_EVENT0("gpu", "GLES2Implementation::TrimResources");
+
+  bool initial_state = aggressively_free_resources_;
+
+  SetAggressivelyFreeResources(true);
+
+  if (!initial_state) {
+    SetAggressivelyFreeResources(false);
+  }
+}
+
 bool GLES2Implementation::OnMemoryDump(
     const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {
@@ -5895,6 +5917,46 @@ bool GLES2Implementation::AnyClientsVisible() const {
   return num_visible_clients_ > 0;
 }
 
+std::unique_ptr<ContextSupport::ScopedBusy>
+GLES2Implementation::ClientBecameBusy() {
+  DCHECK(is_idle_);
+  is_idle_ = false;
+
+  if (idle_callback_) {
+    // We can only modify |pending_idle_callback_| from the provided callback

[danakj, 2016/08/23 01:26:25]

Its kinda awk if you end up running the cancel right after the callback because
of posting order.

What if you used a generation id like "scheduled idle 5" and "cancelled idle 5"
now you don't run 5. but if scheduled idle != cancelled idle then you let the
posted cleanup run? or something like that?

[ericrk, 2016/08/24 18:32:21]

It's actually a little bit tricky to avoid races like this. The problem has to
do with safely accessing the state related to the callback. 

Assuming this is a locking context, at the point when we call
ClientBecameBusy/ClientBecameNotBusy, we are locked by the caller. However, at
the point when the delayed task is run, we are no longer locked. This means we
can't safely access any mutable state that might be in use by another thread.

There are two ways to work around this:
 1) Ensure that any mutable state which affects the callback is *only* modified
from a single thread. This is what we currently do - the callable callback is
the "mutable state", and it is only scheduled/canceled/run on the callback
thread.
 2) Take an additional lock before modifying any callback related mutable state.

As mentioned, the code I initially wrote uses option (1). We could move to a
callback generation id, but modification of this ID would have to happen with
the same restrictions, using either (1) or (2).

If we use a generation id with option (1), we still have the same race where a
cancel can happen right after the callback is run, as we have to post generation
updates to the callback thread.

If we use a generation id with option (2), we need a new lock which is held
around modifications of the generation id, and this is still a bit racy.

The only way I can think of to minimize this race without doing (1) or (2) is to
add an additional "ShouldRunIdleCallback(uint32_t idle_callback_id)" on
ContextSupport. The external callback code could then call this after acquiring
the context lock, and early out if it returns false. This would ensure that
generation id was only checked / modified while the context lock is held.
However, this is pretty roundabout and puts additional checking burden on the
callback.

WDYT?

+    // thread due to CancellableCallback constraints. Move to that thread.
+    idle_callback_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&GLES2Implementation::CancelIdleCallback,
+                              weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  return base::MakeUnique<ScopedBusyImpl>(this);
+}
+
+void GLES2Implementation::ClientBecameNotBusy(
+    std::unique_ptr<ScopedBusy> busy) {
+  DCHECK(busy);
+  DCHECK(!is_idle_);
+  is_idle_ = true;
+
+  if (idle_callback_) {
+    // We can only modify |pending_idle_callback_| from the provided callback
+    // thread due to CancellableCallback constraints. Move to that thread.
+    idle_callback_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&GLES2Implementation::ScheduleIdleCallback,
+                              weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  static_cast<ScopedBusyImpl*>(busy.get())->Release(this);
+}
+
+void GLES2Implementation::SetIdleCallback(
+    const base::Closure& callback,
+    scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner) {
+  idle_callback_ = callback;
+  idle_callback_task_runner_ = std::move(callback_task_runner);
+}
+
 void GLES2Implementation::SetLostContextCallback(
     const base::Closure& callback) {
   lost_context_callback_ = callback;
@@ -6929,6 +6991,22 @@ void GLES2Implementation::InvalidateCachedExtensions() {
   cached_extensions_.clear();
 }
 
+void GLES2Implementation::ScheduleIdleCallback() {
+  if (!idle_callback_)
+    return;
+  if (!pending_idle_callback_)
+    pending_idle_callback_.reset(new base::CancelableClosure());
+
+  pending_idle_callback_->Reset(idle_callback_);
+  idle_callback_task_runner_->PostDelayedTask(
+      FROM_HERE, pending_idle_callback_->callback(), kIdleCallbackDelay);
+}
+
+void GLES2Implementation::CancelIdleCallback() {
+  if (pending_idle_callback_)
+    pending_idle_callback_->Cancel();
+}
+
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.