Creating a framework for suppressing pollution of the profiler data

base/threading/sequenced_worker_pool.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 "base/threading/sequenced_worker_pool.h"
 
 #include <list>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 737 matching lines @@
           SignalHasWork();
           delete_these_outside_lock.clear();
 
           // Complete thread creation outside the lock if necessary.
           if (new_thread_id)
             FinishStartingAdditionalThread(new_thread_id);
 
           this_worker->set_running_task_info(
               SequenceToken(task.sequence_token_id), task.shutdown_behavior);
 
-          tracked_objects::TrackedTime start_time =
-              tracked_objects::ThreadData::NowForStartOfRun(task.birth_tally);
+          tracked_objects::TaskStopwatch stopwatch;
+          stopwatch.Start(
+              tracked_objects::ThreadData::NowForStartOfRun(task.birth_tally));

[jar (doing other things), 2014/09/02 16:52:16]

The reason there is an API called NowForStartOfRun(), instead of just calling
tracked_objects::Now() (a special flavor of Now()) is to make the API concise
(one call, induces side effects, and gets the time value that we need).  We
could have used two function calls... but that would be less concise, an error
prone.

That is indeed also why we have NowForEndOfRun() (gets time, and induces side
effect (of which there are currently none!)).  

That also allowed us to sandwich as tightly as possible the acquisition of the
times around the Run() event we wanted to time.


Given that the StopWatch class is for the exclusive use of the profiler, it can
acquire scoped_objects::Now() directly (no need to even pass in timer values). 
It can always get them as needed from the Now() function.

As a result, the above lines should probably read:
tracked_objects::ThreadData::PrepareForStartOfRun(task.birth_tally);
tracked_objects::TaskStopwatch stopwatch; // Implicitly starts timer.

[vadimt, 2014/09/02 22:41:37]

Done.

+          task.task.Run();
+          stopwatch.Stop(tracked_objects::ThreadData::NowForEndOfRun());

[jar (doing other things), 2014/09/02 16:52:16]

This call can then be simply:
stopwatch.Stop();

It may even be that we don't need to call this, and the first line of the next
method *could* be the call to Stop().

In general, it is nicest to tightly sandwidth the Run() with the start (via
construction?) and the Stop (via explicit call). 

[vadimt, 2014/09/02 22:41:37]

Done.
Given that there are 3 methods that can follow stopwatch.Stop
(TallyRunOnNamedThreadIfTracking, TallyRunOnWorkerThreadIfTracking and
TallyRunInAScopedRegionIfTracking), it's not elegant to call Stop in all 3 of
them. So I kept a separate Stop method.

 
-          task.task.Run();
-
-          tracked_objects::ThreadData::TallyRunOnNamedThreadIfTracking(task,
-              start_time, tracked_objects::ThreadData::NowForEndOfRun());
+          tracked_objects::ThreadData::TallyRunOnNamedThreadIfTracking(
+              task, stopwatch);

[jar (doing other things), 2014/09/02 16:52:16]

If you're willing to embed the Stop() into the above call, we'd instead pass in:
&stopwatch.

The big question is to look at all call sites, and see if there are other
calculations done during the call (as args are passed in).  It would be best to
do the stop very shortly after the task completes.  

[vadimt, 2014/09/02 22:41:36]

See the above answer.

 
           // Make sure our task is erased outside the lock for the
           // same reason we do this with delete_these_oustide_lock.
           // Also, do it before calling set_running_task_info() so
           // that sequence-checking from within the task's destructor
           // still works.
           task.task = Closure();
 
           this_worker->set_running_task_info(
               SequenceToken(), CONTINUE_ON_SHUTDOWN);
@@ skipping 510 matching lines @@
 void SequencedWorkerPool::Shutdown(int max_new_blocking_tasks_after_shutdown) {
   DCHECK(constructor_message_loop_->BelongsToCurrentThread());
   inner_->Shutdown(max_new_blocking_tasks_after_shutdown);
 }
 
 bool SequencedWorkerPool::IsShutdownInProgress() {
   return inner_->IsShutdownInProgress();
 }
 
 }  // namespace base