GPU: Only allow the UI channel to preempt if all stubs are scheduled.

content/common/gpu/gpu_channel.cc

Index: content/common/gpu/gpu_channel.cc
diff --git a/content/common/gpu/gpu_channel.cc b/content/common/gpu/gpu_channel.cc
index c07d15ffc7a03ef590351dc6648a1aaab3444032..d73e2f77cc0b8de87ec5b3f6acb87e510615a894 100644
--- a/content/common/gpu/gpu_channel.cc
+++ b/content/common/gpu/gpu_channel.cc
@@ -157,7 +157,8 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter {
         channel_(NULL),
         sync_point_manager_(sync_point_manager),
         message_loop_(message_loop),
-        messages_received_(0) {
+        messages_received_(0),
+        a_stub_is_descheduled_(false) {
   }
 
   virtual void OnFilterAdded(IPC::Channel* channel) OVERRIDE {
@@ -207,8 +208,21 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter {
     UpdatePreemptionState();
   }
 
-  void SetPreemptingFlag(gpu::PreemptionFlag* preempting_flag) {
+  void SetPreemptingFlagAndSchedulingState(
+      gpu::PreemptionFlag* preempting_flag,
+      bool a_stub_is_descheduled) {
     preempting_flag_ = preempting_flag;
+    a_stub_is_descheduled_ = a_stub_is_descheduled;
+  }
+
+  void UpdateStubSchedulingState(bool a_stub_is_descheduled) {

[piman, 2013/02/27 21:54:43]

I'm afraid this will kick preemption more than you'd generally want. Basically
any time passed unscheduled counts towards the preemption timeout.
OTOH, I think changing the state machine to take a_stub_is_descheduled_ into
account isn't bad at all, all you have to change is UpdatePreemtionState, to
test a_stub_is_descheduled_ any time you check pending_messages_.empty()
(and call UpdateStubSchedulingState)

[jonathan.backer, 2013/02/28 21:43:56]

Done.

+    a_stub_is_descheduled_ = a_stub_is_descheduled;
+    if (preemption_state_ == PREEMPTING) {
+      if (a_stub_is_descheduled_)
+        preempting_flag_->Reset();
+      else
+        preempting_flag_->Set();
+    }
   }
 
  protected:
@@ -228,7 +242,8 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter {
     // We can preempt whenever any IPC processing takes more than
     // kPreemptWaitTimeMs.
     CHECKING,
-    // We are currently preempting.
+    // We are currently preempting if and only if all stubs on this channel
+    // are scheduled.
     PREEMPTING,
   };
 
@@ -324,7 +339,8 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter {
     timer_.Stop();
 
     preemption_state_ = PREEMPTING;
-    preempting_flag_->Set();
+    if (!a_stub_is_descheduled_)
+      preempting_flag_->Set();
     TRACE_COUNTER_ID1("gpu", "GpuChannel::Preempting", this, 1);
 
     timer_.Start(
@@ -380,6 +396,8 @@ class SyncPointMessageFilter : public IPC::ChannelProxy::MessageFilter {
   uint64 messages_received_;
 
   base::OneShotTimer<SyncPointMessageFilter> timer_;
+
+  bool a_stub_is_descheduled_;
 };
 
 GpuChannel::GpuChannel(GpuChannelManager* gpu_channel_manager,
@@ -399,7 +417,8 @@ GpuChannel::GpuChannel(GpuChannelManager* gpu_channel_manager,
       handle_messages_scheduled_(false),
       processed_get_state_fast_(false),
       currently_processing_message_(NULL),
-      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+      a_stub_is_descheduled_(false) {
   DCHECK(gpu_channel_manager);
   DCHECK(client_id);
 
@@ -545,6 +564,26 @@ void GpuChannel::OnScheduled() {
   handle_messages_scheduled_ = true;
 }
 
+void GpuChannel::StubSchedulingChanged() {
+  bool a_stub_is_descheduled = false;
+  for (StubMap::Iterator<GpuCommandBufferStub> it(&stubs_);
+       !it.IsAtEnd(); it.Advance()) {

[piman, 2013/02/27 21:54:43]

Could we count scheduled stubs? If we fix the callbacks so that this is called
exactly once for each change (see below), we can avoid walking the map every
time.

[jonathan.backer, 2013/02/28 21:43:56]

Done. I'm concerned about teardown. I think I've handled that appropriately in
OnDestroyCommandBuffer.

+    if (!it.GetCurrentValue()->IsScheduled()) {
+      a_stub_is_descheduled = true;
+      break;
+    }
+  }
+  if (a_stub_is_descheduled != a_stub_is_descheduled_) {
+    a_stub_is_descheduled_ = a_stub_is_descheduled;
+    if (preempting_flag_.get()) {
+      io_message_loop_->PostTask(
+          FROM_HERE,
+          base::Bind(&SyncPointMessageFilter::UpdateStubSchedulingState,
+                     filter_, a_stub_is_descheduled));
+    }
+  }
+}
+
 void GpuChannel::CreateViewCommandBuffer(
     const gfx::GLSurfaceHandle& window,
     int32 surface_id,
@@ -649,8 +688,8 @@ gpu::PreemptionFlag* GpuChannel::GetPreemptionFlag() {
     preempting_flag_ = new gpu::PreemptionFlag;
     io_message_loop_->PostTask(
         FROM_HERE,
-        base::Bind(&SyncPointMessageFilter::SetPreemptingFlag,
-                   filter_, preempting_flag_));
+        base::Bind(&SyncPointMessageFilter::SetPreemptingFlagAndSchedulingState,
+                   filter_, preempting_flag_, a_stub_is_descheduled_));
   }
   return preempting_flag_.get();
 }