[ThreadPool]: Fix worker thread wakeup churn.

Context: Several worker_threads are woken up. Meanwhile progress is made
and the desired number of workers is reduced.
Problem: the first worker to wakeup will go back to sleep, even if
it could pick up work. This effectively increase latency before work
is picked up.
Expected: a worker that was just woken up should pick up pending work.

Fix: Avoid using GetDesiredNumAwakeWorkersLockRequired() to make
a decision of going back to sleep. |max_tasks_| is used instead.
If a worker is active and sees available work, it should *always* pick
it up, unless prevented by |max_tasks_|.

Drive by: call CanGetWorkLockRequired for early exit. There's no
point attempting to wake up additional workers if the current worker
was going back to sleep.


Change-Id: I7c7243046372ec99444320256f2df8d32e0d07a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568903
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834258}

base/task/thread_pool/thread_group_impl.cc

@@ -585,6 +585,9 @@ RegisteredTaskSource ThreadGroupImpl::WorkerThreadDelegateImpl::GetWork(
 
   DCHECK(ContainsWorker(outer_->workers_, worker));
 
+  if (!CanGetWorkLockRequired(&executor, worker))
+    return nullptr;
+
   // Use this opportunity, before assigning work to this worker, to create/wake
   // additional workers if needed (doing this here allows us to reduce
   // potentially expensive create/wake directly on PostTask()).
@@ -595,9 +598,6 @@ RegisteredTaskSource ThreadGroupImpl::WorkerThreadDelegateImpl::GetWork(
     executor.FlushWorkerCreation(&outer_->lock_);
   }
 
-  if (!CanGetWorkLockRequired(&executor, worker))
-    return nullptr;
-
   RegisteredTaskSource task_source;
   TaskPriority priority;
   while (!task_source && !outer_->priority_queue_.IsEmpty()) {
@@ -893,8 +893,7 @@ bool ThreadGroupImpl::WorkerThreadDelegateImpl::CanGetWorkLockRequired(
   // max tasks increases). This ensures that if we have excess workers in the
   // thread group, they get a chance to no longer be excess before being cleaned
   // up.
-  if (outer_->GetNumAwakeWorkersLockRequired() >
-      outer_->GetDesiredNumAwakeWorkersLockRequired()) {
+  if (outer_->GetNumAwakeWorkersLockRequired() > outer_->max_tasks_) {
     OnWorkerBecomesIdleLockRequired(worker);
     return false;
   }