Use TRACE_TASK_EXECUTION in SequencedWorkerPool.

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 <stdint.h>
 
 #include <list>
 #include <map>
@@ skipping 17 matching lines @@
 #include "base/synchronization/condition_variable.h"
 #include "base/synchronization/lock.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/task_scheduler/task_scheduler.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
 #include "base/threading/thread_local.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
-#include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/trace_event.h"
 #include "base/tracked_objects.h"
 #include "base/tracking_info.h"
 #include "build/build_config.h"
 
 #if defined(OS_MACOSX)
 #include "base/mac/scoped_nsautorelease_pool.h"
 #elif defined(OS_WIN)
 #include "base/win/scoped_com_initializer.h"
 #endif
@@ skipping 936 matching lines @@
 
       HandleCleanup();
 
       // See GetWork for what delete_these_outside_lock is doing.
       SequencedTask task;
       TimeDelta wait_time;
       std::vector<Closure> delete_these_outside_lock;
       GetWorkStatus status =
           GetWork(&task, &wait_time, &delete_these_outside_lock);
       if (status == GET_WORK_FOUND) {
-        TRACE_EVENT_WITH_FLOW2(TRACE_DISABLED_BY_DEFAULT("toplevel.flow"),
-            "SequencedWorkerPool::Inner::ThreadLoop",
+        TRACE_TASK_EXECUTION("SequencedWorkerPool::Inner::ThreadLoop", task);

[danakj, 2016/10/20 21:05:18]

Is reordering these ok? They will not nest differently?

[fdoray, 2016/10/21 12:47:59]

Yes.

This new ordering is consistent with MessageLoop and TaskScheduler.

MessageLoop snippet:
https://cs.chromium.org/chromium/src/base/message_loop/message_loop.cc?sq=pac...

  // Note: This invokes TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION.
  TRACE_TASK_EXECUTION("MessageLoop::RunTask", *pending_task);

  for (auto& observer : task_observers_)
    observer.WillProcessTask(*pending_task);
  // Note: RunTask invokes TRACE_EVENT_WITH_FLOW.
  task_annotator_.RunTask("MessageLoop::PostTask", pending_task);
  for (auto& observer : task_observers_)
    observer.DidProcessTask(*pending_task);

TaskScheduler snippet:
https://cs.chromium.org/chromium/src/base/task_scheduler/task_tracker.cc?sq=p...


  // Note: This invokes TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION.
  TRACE_TASK_EXECUTION(kRunFunctionName, *task);

  ...    

  // Note: RunTask invokes TRACE_EVENT_WITH_FLOW.
  task_annotator.RunTask(kQueueFunctionName, task.get());

+        TRACE_EVENT_WITH_FLOW0(TRACE_DISABLED_BY_DEFAULT("toplevel.flow"),
+            "SequencedWorkerPool::Inner::PostTask",

[danakj, 2016/10/20 21:05:18]

This is ThreadLoop tho?

[fdoray, 2016/10/21 12:47:59]

No.

To be consistent with MessageLoop and TaskScheduler, the name of the post task
function must be used with the TRACE_EVENT_WITH_FLOW0 macro
https://cs.chromium.org/chromium/src/base/debug/task_annotator.cc?sq=package:...
and the name of the run task function must be used with the TRACE_TASK_EXECUTION
function (see previous comment in this file).

             TRACE_ID_MANGLE(GetTaskTraceID(task, static_cast<void*>(this))),
-            TRACE_EVENT_FLAG_FLOW_IN,
-            "src_file", task.posted_from.file_name(),
-            "src_func", task.posted_from.function_name());
-        TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION task_event(

[gab, 2016/10/18 18:15:02]

Why was this removed?

[fdoray, 2016/10/18 19:19:29]

This is done by TRACE_TASK_EXECUTION
https://cs.chromium.org/chromium/src/base/trace_event/trace_event.h?l=394

-            task.posted_from.file_name());
+            TRACE_EVENT_FLAG_FLOW_IN);
         int new_thread_id = WillRunWorkerTask(task);
         {
           AutoUnlock unlock(lock_);
           // There may be more work available, so wake up another
           // worker thread. (Technically not required, since we
           // already get a signal for each new task, but it doesn't
           // hurt.)
           SignalHasWork();
           delete_these_outside_lock.clear();
 
@@ skipping 608 matching lines @@
 bool SequencedWorkerPool::IsShutdownInProgress() {
   return inner_->IsShutdownInProgress();
 }
 
 bool SequencedWorkerPool::IsRunningSequenceOnCurrentThread(
     SequenceToken sequence_token) const {
   return inner_->IsRunningSequenceOnCurrentThread(sequence_token);
 }
 
 }  // namespace base