Fix a bug in the implementation of DescheduleUntilFinishedCHROMIUM.

gpu/ipc/service/gpu_command_buffer_stub.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 "gpu/ipc/service/gpu_command_buffer_stub.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 286 matching lines @@
 void GpuCommandBufferStub::PerformWork() {
   TRACE_EVENT0("gpu", "GpuCommandBufferStub::PerformWork");
 
   FastSetActiveURL(active_url_, active_url_hash_, channel_);
   if (decoder_.get() && !MakeCurrent())
     return;
 
   if (executor_) {
     uint32_t current_unprocessed_num =
         channel()->gpu_channel_manager()->GetUnprocessedOrderNum();
-    // We're idle when no messages were processed or scheduled.
+    // We're idle when no messages were processed or scheduled, or if the
+    // executor is descheduled.
     bool is_idle = (previous_processed_num_ == current_unprocessed_num);
+    is_idle |= !executor_->scheduled();

[piman, 2016/06/10 21:52:27]

Trying to think a bit more about this...

We want to do idle work when nothing else is executing. The check on the
unprocessed message is global - looking at all channels, all command buffers,
making sure we don't have any unprocessed message.

With this change, we would be spending cycles trying to perform idle work on any
command buffer that is unscheduled, even if something else (other command
buffer), possibly higher priority, would be running otherwise - e.g. in case of
a WaitSyncToken.

So what we would really want is: do idle work if either:
1- there's nothing else to do (no unprocessed message), or
2- every command buffer that has messages to process is unscheduled (assuming,
the idle work will eventually reschedule it)

This is something that Sunny's global GPU scheduler should make easy,
unfortunately it has not landed yet.


In the mean time my suggestion is to implement something similar to
GpuChannelManager::GetUnprocessedOrderNum(): iterate over all the channels, and
in each channel iterate over all the streams (GpuChannelMessageQueue), and check
scheduled_.
If they're all false, then we can run idle work. WDYT?

[erikchen, 2016/06/22 19:23:07]

I think that your proposed solution still has edge cases. If there is a tiny
amount of unprocessed work each time a descheduled executor tries to reschedule
itself, it will never be rescheduled. In the more common case, it will
reschedule itself later than it otherwise should have been rescheduled.

Perhaps this is a nomenclature problem? Attempting to reschedule the executor is
a more urgent priority than idle work. I could add some additional logic and
rename some methods to check for "mandatory work"?

[piman, 2016/06/22 21:47:41]

Yeah, I guess we should separate "polling work", which blocks forwards progress
from "idle work", which doesn't block forward progress and that we should only
do if nothing else wants to run.
I.e. for the purpose of rescheduling, it should behave like the pending queries
rather than idle work.

     if (!is_idle && !last_idle_time_.is_null()) {
       base::TimeDelta time_since_idle =
           base::TimeTicks::Now() - last_idle_time_;
       base::TimeDelta max_time_since_idle =
           base::TimeDelta::FromMilliseconds(kMaxTimeSinceIdleMs);
 
       // Force idle when it's been too long since last time we were idle.
       if (time_since_idle > max_time_since_idle)
         is_idle = true;
     }
@@ skipping 794 matching lines @@
   Send(new GpuCommandBufferMsg_SwapBuffersCompleted(route_id_, params));
 }
 
 void GpuCommandBufferStub::SendUpdateVSyncParameters(base::TimeTicks timebase,
                                                      base::TimeDelta interval) {
   Send(new GpuCommandBufferMsg_UpdateVSyncParameters(route_id_, timebase,
                                                      interval));
 }
 
 }  // namespace gpu