Commit 9cfdee0a authored by Katie Dillon's avatar Katie Dillon Committed by Commit Bot

[scheduling] Remove unecessary references to TaskQueue in metrics.

A few places in the scheduling code take a queue as an argument to
recording metrics but don't actually use the queue for anything. This
CL removes those references.

This CL is also a pre-requiste for crrev.com/c/2453591 which refactors
MainThreadTaskQueue to not extend TaskQueue. Removing these unecessary
references makes that refactor simpler.

Change-Id: Ie98e19e44be2e5d156981a775c757f97766d0c35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486214
Commit-Queue: Katie Dillon <kdillon@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819858}
parent 7afefc29
...@@ -34,7 +34,6 @@ ThreadMetrics::ThreadMetrics(ThreadType thread_type, ...@@ -34,7 +34,6 @@ ThreadMetrics::ThreadMetrics(ThreadType thread_type,
ThreadMetrics::~ThreadMetrics() {} ThreadMetrics::~ThreadMetrics() {}
bool ThreadMetrics::ShouldDiscardTask( bool ThreadMetrics::ShouldDiscardTask(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) { const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
// TODO(altimin): Investigate the relationship between thread time and // TODO(altimin): Investigate the relationship between thread time and
...@@ -43,7 +42,6 @@ bool ThreadMetrics::ShouldDiscardTask( ...@@ -43,7 +42,6 @@ bool ThreadMetrics::ShouldDiscardTask(
} }
void ThreadMetrics::RecordTaskMetrics( void ThreadMetrics::RecordTaskMetrics(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) { const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
DCHECK(!has_cpu_timing_for_each_task_ || task_timing.has_thread_time()); DCHECK(!has_cpu_timing_for_each_task_ || task_timing.has_thread_time());
......
...@@ -25,13 +25,11 @@ class COMPONENT_EXPORT(SCHEDULING_METRICS) ThreadMetrics { ...@@ -25,13 +25,11 @@ class COMPONENT_EXPORT(SCHEDULING_METRICS) ThreadMetrics {
~ThreadMetrics(); ~ThreadMetrics();
bool ShouldDiscardTask( bool ShouldDiscardTask(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing); const base::sequence_manager::TaskQueue::TaskTiming& task_timing);
// Record task metrics which are shared between threads. // Record task metrics which are shared between threads.
void RecordTaskMetrics( void RecordTaskMetrics(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing); const base::sequence_manager::TaskQueue::TaskTiming& task_timing);
......
...@@ -40,19 +40,16 @@ TEST(MetricsHelperTest, TaskDurationPerThreadType) { ...@@ -40,19 +40,16 @@ TEST(MetricsHelperTest, TaskDurationPerThreadType) {
false /* has_cpu_timing_for_each_task */); false /* has_cpu_timing_for_each_task */);
main_thread_metrics.RecordTaskMetrics( main_thread_metrics.RecordTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(0),
FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(0), ThreadSeconds(15)));
ThreadSeconds(15)));
compositor_metrics.RecordTaskMetrics( compositor_metrics.RecordTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(80), ThreadSeconds(0),
FakeTaskTiming(Seconds(10), Seconds(80), ThreadSeconds(0), ThreadSeconds(5)));
ThreadSeconds(5)));
compositor_metrics.RecordTaskMetrics( compositor_metrics.RecordTaskMetrics(
nullptr, FakeTask(), FakeTaskTiming(Seconds(100), Seconds(200))); FakeTask(), FakeTaskTiming(Seconds(100), Seconds(200)));
worker_metrics.RecordTaskMetrics( worker_metrics.RecordTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(125), ThreadSeconds(0),
FakeTaskTiming(Seconds(10), Seconds(125), ThreadSeconds(0), ThreadSeconds(25)));
ThreadSeconds(25)));
EXPECT_THAT( EXPECT_THAT(
histogram_tester.GetAllSamples( histogram_tester.GetAllSamples(
...@@ -84,13 +81,11 @@ TEST(MetricsHelperTest, TrackedCPUTimeMetrics) { ...@@ -84,13 +81,11 @@ TEST(MetricsHelperTest, TrackedCPUTimeMetrics) {
true /* has_cpu_timing_for_each_task */); true /* has_cpu_timing_for_each_task */);
main_thread_metrics.RecordTaskMetrics( main_thread_metrics.RecordTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(5),
FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(5), ThreadSeconds(15)));
ThreadSeconds(15)));
main_thread_metrics.RecordTaskMetrics( main_thread_metrics.RecordTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(20),
FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(20), ThreadSeconds(25)));
ThreadSeconds(25)));
EXPECT_THAT(histogram_tester.GetAllSamples( EXPECT_THAT(histogram_tester.GetAllSamples(
"Scheduler.Experimental.CPUTimePerThread.Tracked"), "Scheduler.Experimental.CPUTimePerThread.Tracked"),
......
...@@ -71,7 +71,6 @@ MetricsHelper::MetricsHelper(ThreadType thread_type, ...@@ -71,7 +71,6 @@ MetricsHelper::MetricsHelper(ThreadType thread_type,
MetricsHelper::~MetricsHelper() {} MetricsHelper::~MetricsHelper() {}
bool MetricsHelper::ShouldDiscardTask( bool MetricsHelper::ShouldDiscardTask(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) { const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
// TODO(altimin): Investigate the relationship between thread time and // TODO(altimin): Investigate the relationship between thread time and
...@@ -82,10 +81,9 @@ bool MetricsHelper::ShouldDiscardTask( ...@@ -82,10 +81,9 @@ bool MetricsHelper::ShouldDiscardTask(
} }
void MetricsHelper::RecordCommonTaskMetrics( void MetricsHelper::RecordCommonTaskMetrics(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) { const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
thread_metrics_.RecordTaskMetrics(queue, task, task_timing); thread_metrics_.RecordTaskMetrics(task, task_timing);
thread_task_duration_reporter_.RecordTask(thread_type_, thread_task_duration_reporter_.RecordTask(thread_type_,
task_timing.wall_duration()); task_timing.wall_duration());
......
...@@ -44,13 +44,11 @@ class PLATFORM_EXPORT MetricsHelper { ...@@ -44,13 +44,11 @@ class PLATFORM_EXPORT MetricsHelper {
protected: protected:
bool ShouldDiscardTask( bool ShouldDiscardTask(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing); const base::sequence_manager::TaskQueue::TaskTiming& task_timing);
// Record task metrics which are shared between threads. // Record task metrics which are shared between threads.
void RecordCommonTaskMetrics( void RecordCommonTaskMetrics(
base::sequence_manager::TaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing); const base::sequence_manager::TaskQueue::TaskTiming& task_timing);
......
...@@ -51,19 +51,16 @@ TEST(MetricsHelperTest, TaskDurationPerThreadType) { ...@@ -51,19 +51,16 @@ TEST(MetricsHelperTest, TaskDurationPerThreadType) {
false /* has_cpu_timing_for_each_task */); false /* has_cpu_timing_for_each_task */);
main_thread_metrics.RecordCommonTaskMetrics( main_thread_metrics.RecordCommonTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(0),
FakeTaskTiming(Seconds(10), Seconds(50), ThreadSeconds(0), ThreadSeconds(15)));
ThreadSeconds(15)));
compositor_metrics.RecordCommonTaskMetrics( compositor_metrics.RecordCommonTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(80), ThreadSeconds(0),
FakeTaskTiming(Seconds(10), Seconds(80), ThreadSeconds(0), ThreadSeconds(5)));
ThreadSeconds(5)));
compositor_metrics.RecordCommonTaskMetrics( compositor_metrics.RecordCommonTaskMetrics(
nullptr, FakeTask(), FakeTaskTiming(Seconds(100), Seconds(200))); FakeTask(), FakeTaskTiming(Seconds(100), Seconds(200)));
worker_metrics.RecordCommonTaskMetrics( worker_metrics.RecordCommonTaskMetrics(
nullptr, FakeTask(), FakeTask(), FakeTaskTiming(Seconds(10), Seconds(125), ThreadSeconds(0),
FakeTaskTiming(Seconds(10), Seconds(125), ThreadSeconds(0), ThreadSeconds(25)));
ThreadSeconds(25)));
EXPECT_THAT( EXPECT_THAT(
histogram_tester.GetAllSamples( histogram_tester.GetAllSamples(
......
...@@ -241,10 +241,10 @@ void MainThreadMetricsHelper::RecordTaskMetrics( ...@@ -241,10 +241,10 @@ void MainThreadMetricsHelper::RecordTaskMetrics(
MainThreadTaskQueue* queue, MainThreadTaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) { const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
if (ShouldDiscardTask(queue, task, task_timing)) if (ShouldDiscardTask(task, task_timing))
return; return;
MetricsHelper::RecordCommonTaskMetrics(queue, task, task_timing); MetricsHelper::RecordCommonTaskMetrics(task, task_timing);
total_task_time_reporter_.RecordAdditionalDuration( total_task_time_reporter_.RecordAdditionalDuration(
task_timing.wall_duration()); task_timing.wall_duration());
......
...@@ -15,13 +15,12 @@ CompositorMetricsHelper::CompositorMetricsHelper( ...@@ -15,13 +15,12 @@ CompositorMetricsHelper::CompositorMetricsHelper(
CompositorMetricsHelper::~CompositorMetricsHelper() {} CompositorMetricsHelper::~CompositorMetricsHelper() {}
void CompositorMetricsHelper::RecordTaskMetrics( void CompositorMetricsHelper::RecordTaskMetrics(
NonMainThreadTaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) { const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
if (ShouldDiscardTask(queue, task, task_timing)) if (ShouldDiscardTask(task, task_timing))
return; return;
MetricsHelper::RecordCommonTaskMetrics(queue, task, task_timing); MetricsHelper::RecordCommonTaskMetrics(task, task_timing);
} }
} // namespace scheduler } // namespace scheduler
......
...@@ -17,7 +17,6 @@ class PLATFORM_EXPORT CompositorMetricsHelper : public MetricsHelper { ...@@ -17,7 +17,6 @@ class PLATFORM_EXPORT CompositorMetricsHelper : public MetricsHelper {
~CompositorMetricsHelper(); ~CompositorMetricsHelper();
void RecordTaskMetrics( void RecordTaskMetrics(
NonMainThreadTaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing); const base::sequence_manager::TaskQueue::TaskTiming& task_timing);
......
...@@ -56,8 +56,7 @@ void CompositorThreadScheduler::OnTaskCompleted( ...@@ -56,8 +56,7 @@ void CompositorThreadScheduler::OnTaskCompleted(
base::sequence_manager::TaskQueue::TaskTiming* task_timing, base::sequence_manager::TaskQueue::TaskTiming* task_timing,
base::sequence_manager::LazyNow* lazy_now) { base::sequence_manager::LazyNow* lazy_now) {
task_timing->RecordTaskEnd(lazy_now); task_timing->RecordTaskEnd(lazy_now);
compositor_metrics_helper_.RecordTaskMetrics(worker_task_queue, task, compositor_metrics_helper_.RecordTaskMetrics(task, *task_timing);
*task_timing);
} }
scoped_refptr<scheduler::SingleThreadIdleTaskRunner> scoped_refptr<scheduler::SingleThreadIdleTaskRunner>
......
...@@ -29,13 +29,12 @@ void WorkerMetricsHelper::SetParentFrameType(FrameOriginType frame_type) { ...@@ -29,13 +29,12 @@ void WorkerMetricsHelper::SetParentFrameType(FrameOriginType frame_type) {
} }
void WorkerMetricsHelper::RecordTaskMetrics( void WorkerMetricsHelper::RecordTaskMetrics(
NonMainThreadTaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) { const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
if (ShouldDiscardTask(queue, task, task_timing)) if (ShouldDiscardTask(task, task_timing))
return; return;
MetricsHelper::RecordCommonTaskMetrics(queue, task, task_timing); MetricsHelper::RecordCommonTaskMetrics(task, task_timing);
bool backgrounded = internal::ProcessState::Get()->is_process_backgrounded; bool backgrounded = internal::ProcessState::Get()->is_process_backgrounded;
......
...@@ -21,7 +21,6 @@ class PLATFORM_EXPORT WorkerMetricsHelper : public MetricsHelper { ...@@ -21,7 +21,6 @@ class PLATFORM_EXPORT WorkerMetricsHelper : public MetricsHelper {
~WorkerMetricsHelper(); ~WorkerMetricsHelper();
void RecordTaskMetrics( void RecordTaskMetrics(
NonMainThreadTaskQueue* queue,
const base::sequence_manager::Task& task, const base::sequence_manager::Task& task,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing); const base::sequence_manager::TaskQueue::TaskTiming& task_timing);
......
...@@ -207,7 +207,7 @@ void WorkerThreadScheduler::OnTaskCompleted( ...@@ -207,7 +207,7 @@ void WorkerThreadScheduler::OnTaskCompleted(
PerformMicrotaskCheckpoint(); PerformMicrotaskCheckpoint();
task_timing->RecordTaskEnd(lazy_now); task_timing->RecordTaskEnd(lazy_now);
worker_metrics_helper_.RecordTaskMetrics(task_queue, task, *task_timing); worker_metrics_helper_.RecordTaskMetrics(task, *task_timing);
if (task_queue_throttler_) { if (task_queue_throttler_) {
task_queue_throttler_->OnTaskRunTimeReported( task_queue_throttler_->OnTaskRunTimeReported(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment