Add TaskScheduler.NumTasksBeforeDetach.[worker pool name] histogram.

base/task_scheduler/scheduler_worker_pool_impl.cc

Index: base/task_scheduler/scheduler_worker_pool_impl.cc
diff --git a/base/task_scheduler/scheduler_worker_pool_impl.cc b/base/task_scheduler/scheduler_worker_pool_impl.cc
index 19833c812d9819c625d980725d5992b5c3e7e07c..f14e5f9249a550e102cfa72ef2944ad749fcf113 100644
--- a/base/task_scheduler/scheduler_worker_pool_impl.cc
+++ b/base/task_scheduler/scheduler_worker_pool_impl.cc
@@ -34,6 +34,8 @@ namespace {
 constexpr char kPoolNameSuffix[] = "Pool";
 constexpr char kDetachDurationHistogramPrefix[] =
     "TaskScheduler.DetachDuration.";
+constexpr char kNumTasksBeforeDetachHistogramPrefix[] =
+    "TaskScheduler.NumTasksBeforeDetach.";
 constexpr char kNumTasksBetweenWaitsHistogramPrefix[] =
     "TaskScheduler.NumTasksBetweenWaits.";
 constexpr char kTaskLatencyHistogramPrefix[] = "TaskScheduler.TaskLatency.";
@@ -280,6 +282,10 @@ class SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl
   // TaskScheduler.NumTasksBetweenWaits histogram was recorded.
   size_t num_tasks_since_last_wait_ = 0;
 
+  // Number of tasks executed since the last time the
+  // TaskScheduler.NumTasksBeforeDetach histogram was recorded.
+  size_t num_tasks_since_last_detach_ = 0;
+
   subtle::Atomic32 num_single_threaded_runners_ = 0;
 
   const int index_;
@@ -486,8 +492,12 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::OnMainEntry(
 
   DCHECK_EQ(num_tasks_since_last_wait_, 0U);
 
+  // Record histograms if the worker detached in the past.
   if (!detach_duration.is_max()) {
     outer_->detach_duration_histogram_->AddTime(detach_duration);
+    outer_->num_tasks_before_detach_histogram_->Add(

[gab, 2016/10/17 18:24:00]

Recording here will only record if the thread is recreated. This seems weird to
me. Why not record on detach?

[robliao, 2016/10/17 23:05:39]

The SchedulerWorker is free to ignore the detach request (e.g. race condition
where a wakeup is pending while the scheduler worker gets ready to detach).

[gab, 2016/10/18 14:45:45]

Right but we could still add SchedulerWorkerDelegate::OnDetach() and call it at
the end of a successful SchedulerWorker::Detach().

The ideal detach is one where we never re-attach and in this case the metric
will ignore those and thus be incorrectly skewed IMO.

[fdoray, 2016/10/18 19:41:22]

Done.

+        num_tasks_since_last_detach_);
+    num_tasks_since_last_detach_ = 0;
     did_detach_since_last_get_work_ = true;
   }
 
@@ -593,6 +603,7 @@ void SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::
     DidRunTaskWithPriority(TaskPriority task_priority,
                            const TimeDelta& task_latency) {
   ++num_tasks_since_last_wait_;
+  ++num_tasks_since_last_detach_;
 
   const int priority_index = static_cast<int>(task_priority);
 
@@ -673,6 +684,16 @@ SchedulerWorkerPoolImpl::SchedulerWorkerPoolImpl(
           TimeDelta::FromHours(1),
           50,
           HistogramBase::kUmaTargetedHistogramFlag)),
+      // Mimics the UMA_HISTOGRAM_COUNTS_1000 macro. A SchedulerWorker is
+      // expected to run between zero and a few hundreds of tasks before
+      // detaching. When it runs more than 1000 tasks, there is no need to know
+      // the exact number of tasks that ran.

[gab, 2016/10/17 18:24:00]

While I agree that once it runs more than a 1000 tasks we can call that a "good
run for the money" and not care how many exactly (i.e. it was worth it to wake
it up).

I don't think it's true that "a SW is expected to run between zero and a few
hundreds of tasks before detaching".

[fdoray, 2016/10/18 19:41:22]

Done.

+      num_tasks_before_detach_histogram_(Histogram::FactoryGet(
+          kNumTasksBeforeDetachHistogramPrefix + name_ + kPoolNameSuffix,
+          1,
+          1000,
+          50,
+          HistogramBase::kUmaTargetedHistogramFlag)),
       // Mimics the UMA_HISTOGRAM_COUNTS_100 macro. A SchedulerWorker is
       // expected to run between zero and a few tens of tasks between waits.
       // When it runs more than 100 tasks, there is no need to know the exact