|
|
DescriptionUse TRACE_TASK_EXECUTION in SequencedWorkerPool.
With this CL, SequencedWorkerPool generates an event in the "toplevel"
category when it runs a task. This is the same behavior as MessageLoop,
TaskScheduler and WorkerPool.
BUG=611818
Committed: https://crrev.com/af4072ecf86c2c8ff7ec217e4830e2c2e5964cf7
Cr-Commit-Position: refs/heads/master@{#426994}
Patch Set 1 #Patch Set 2 : remove unused include #
Total comments: 6
Messages
Total messages: 16 (5 generated)
fdoray@chromium.org changed reviewers: + danakj@chromium.org, gab@chromium.org
PTAL
lgtm w/ question https://chromiumcodereview.appspot.com/2429863002/diff/20001/base/threading/s... File base/threading/sequenced_worker_pool.cc (left): https://chromiumcodereview.appspot.com/2429863002/diff/20001/base/threading/s... base/threading/sequenced_worker_pool.cc:1001: TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION task_event( Why was this removed?
danakj@: PTAL https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... File base/threading/sequenced_worker_pool.cc (left): https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... base/threading/sequenced_worker_pool.cc:1001: TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION task_event( On 2016/10/18 18:15:02, gab wrote: > Why was this removed? This is done by TRACE_TASK_EXECUTION https://cs.chromium.org/chromium/src/base/trace_event/trace_event.h?l=394
https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... File base/threading/sequenced_worker_pool.cc (right): https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... base/threading/sequenced_worker_pool.cc:994: TRACE_TASK_EXECUTION("SequencedWorkerPool::Inner::ThreadLoop", task); Is reordering these ok? They will not nest differently? https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... base/threading/sequenced_worker_pool.cc:996: "SequencedWorkerPool::Inner::PostTask", This is ThreadLoop tho?
danakj@: PTAnL https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... File base/threading/sequenced_worker_pool.cc (right): https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... base/threading/sequenced_worker_pool.cc:994: TRACE_TASK_EXECUTION("SequencedWorkerPool::Inner::ThreadLoop", task); On 2016/10/20 21:05:18, danakj wrote: > Is reordering these ok? They will not nest differently? 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()); https://codereview.chromium.org/2429863002/diff/20001/base/threading/sequence... base/threading/sequenced_worker_pool.cc:996: "SequencedWorkerPool::Inner::PostTask", On 2016/10/20 21:05:18, danakj wrote: > This is ThreadLoop tho? 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).
Thanks LGTM
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Use TRACE_TASK_EXECUTION in SequencedWorkerPool. With this CL, SequencedWorkerPool generates an event in the "toplevel" category when it runs a task. This is the same behavior as MessageLoop, TaskScheduler and WorkerPool. BUG=611818 ========== to ========== Use TRACE_TASK_EXECUTION in SequencedWorkerPool. With this CL, SequencedWorkerPool generates an event in the "toplevel" category when it runs a task. This is the same behavior as MessageLoop, TaskScheduler and WorkerPool. BUG=611818 Committed: https://crrev.com/af4072ecf86c2c8ff7ec217e4830e2c2e5964cf7 Cr-Commit-Position: refs/heads/master@{#426994} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/af4072ecf86c2c8ff7ec217e4830e2c2e5964cf7 Cr-Commit-Position: refs/heads/master@{#426994} |