TaskScheduler: Bump thread priority during shutdown.

base/task_scheduler/scheduler_worker.cc

 // Copyright 2016 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 "base/task_scheduler/scheduler_worker.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 10 matching lines @@
   static std::unique_ptr<Thread> Create(SchedulerWorker* outer) {
     std::unique_ptr<Thread> thread(new Thread(outer));
     thread->Initialize();
     if (thread->thread_handle_.is_null())
       return nullptr;
     return thread;
   }
 
   // PlatformThread::Delegate.
   void ThreadMain() override {
+    // True once the thread's priority has been bumped to accelerate shutdown.
+    bool thread_priority_bumped_for_shutdown = false;

[robliao, 2016/07/19 22:41:28]

I wonder if it might be more extensible to keep track of the current priority,
and that would change your if statement below to...

if (outer_->task_tracker->IsShutdownInProgress() &&
    thread_priority_ != ThreadPriority::NORMAL) {
  thread_priority_ = ThreadPriority::NORMAL;
  PlatformThread::SetCurrentThreadPriority(ThreadPriority::NORMAL);
}

If GetCurrentPriority is fast, I'd go with that, but I speculate it comes with
non-trivial cost.

[fdoray, 2016/07/20 18:22:21]

I like the idea of keeping track of |thread_priority_|. Not sure why the speed
of GetCurrentPriority matters?

FYI, some benchmarks:

Windows - GetCurrentThreadPriority: 127ns
Windows - SetCurrentThreadPriority: 124ns
Mac - GetCurrentThreadPriority: 68ns
Mac - SetCurrentThreadPriority: 936ns

class Timer {
 public:
  ~Timer() {
    const TimeDelta elapsed_time = TimeTicks::Now() - start;
    LOG(ERROR) << "Elapsed time: " << elapsed_time;
  }

 private:
  const TimeTicks start = TimeTicks::Now();
};

constexpr size_t kNumIterations = 100000000;
volatile int counter = 0;

TEST(ThreadPriorityBenchmark, Counter) {
  Timer timer;
  for (size_t i = 0; i < kNumIterations; ++i) {
    ++counter;
  }
}

TEST(ThreadPriorityBenchmark, SetCurrentThreadPriority) {
  Timer timer;
  for (size_t i = 0; i < kNumIterations; ++i) {
    PlatformThread::SetCurrentThreadPriority(ThreadPriority::NORMAL);
    ++counter;
  }
}

TEST(ThreadPriorityBenchmark, GetCurrentThreadPriority) {
  Timer timer;
  for (size_t i = 0; i < kNumIterations; ++i) {
    PlatformThread::GetCurrentThreadPriority();
    ++counter;
  }
}

[robliao, 2016/07/20 19:14:57]

If GetCurrentThreadPriority is nearly as fast as reading the variable, we can
just do that instead of storing the state ourselves (and opening the door to
potential synchronization bugs).

[gab, 2016/07/20 20:30:09]

I much prefer a local variable to a call to the OS here.

[fdoray, 2016/07/20 20:30:37]

GetCurrentThreadPriority() is fast but not sure that 127ns is "nearly as fast"
as reading a variable that can be in cache. Do you think we should still use it?

[robliao, 2016/07/20 22:31:48]

This state is already stored in the TEB, so I'd rather reference that than
duplicate the state here. 127ns is a bit too long for my liking, so let's move
the SetCurrentThreadPriority call and actual_thread_priority_ update into a
separate method to avoid desynchronization. 

[fdoray, 2016/07/21 13:21:56]

Let me know if I understood your comment correctly.

[robliao, 2016/07/21 17:05:51]

Yup! Thanks.

+
     // Set if this thread was detached.
     std::unique_ptr<Thread> detached_thread;
 
     outer_->delegate_->OnMainEntry(outer_);
 
     // A SchedulerWorker starts out waiting for work.
     WaitForWork();
 
     while (!outer_->task_tracker_->IsShutdownComplete() &&
            !outer_->ShouldExitForTesting()) {
       DCHECK(outer_);
+
+      // Bump the thread's priority during shutdown.
+      if (outer_->thread_priority_ == ThreadPriority::BACKGROUND &&
+          !thread_priority_bumped_for_shutdown &&
+          outer_->task_tracker_->IsShutdownInProgress()) {
+        PlatformThread::SetCurrentThreadPriority(ThreadPriority::NORMAL);
+        thread_priority_bumped_for_shutdown = true;
+      }
+
       // Get the sequence containing the next task to execute.
       scoped_refptr<Sequence> sequence = outer_->delegate_->GetWork(outer_);
       if (!sequence) {
         if (outer_->delegate_->CanDetach(outer_)) {
           detached_thread = outer_->Detach();
           if (detached_thread) {
             DCHECK_EQ(detached_thread.get(), this);
             PlatformThread::Detach(thread_handle_);
             outer_ = nullptr;
             break;
@@ skipping 161 matching lines @@
   CreateThread();
 }
 
 bool SchedulerWorker::ShouldExitForTesting() const {
   AutoSchedulerLock auto_lock(should_exit_for_testing_lock_);
   return should_exit_for_testing_;
 }
 
 }  // namespace internal
 }  // namespace base