Commit adafd271 authored by Alexander Timin's avatar Alexander Timin Committed by Commit Bot

[message_loop] Remove MessageLoop.DelayedTaskQueueForUI.PendingTasksCountOnIdle metric

MessageLoop is going away and this diagnosis metric will need to be re-evaluated.

For the record, this metric's latest values were:

                   Median     99th'ile   Note
Windows Stable :     37          448      >50% of users hit 500+ >=1 times a day
Windows Canary:      37          258      >50% of users hit 250+ >=1 times a day
Android Stable:      28          113      >50% of users hit 65+ >= 1 times a day
Android Dev:         28          149      >50% of users hit 86+ >=1 times a day

R=gab@chromium.org
BUG=860801,891670

Change-Id: Ib45c47286a2a9153827bc4f6e71c7d056e90a5f0
Reviewed-on: https://chromium-review.googlesource.com/c/1333752Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Commit-Queue: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607936}
parent 9ecc4552
......@@ -190,14 +190,6 @@ MessageLoop::MessageLoop(Type type, MessagePumpFactoryCallback pump_factory)
// Bound in BindToCurrentThread();
DETACH_FROM_THREAD(bound_thread_checker_);
if (type_ == MessageLoop::TYPE_UI) {
// Only track idle metrics in MessageLoopForUI to avoid too much contention
// logging the histogram (https://crbug.com/860801) -- there's typically
// only one UI thread per process and, for practical purposes, restricting
// the MessageLoop diagnostic metrics to it yields similar information.
message_loop_impl_->SetShouldRecordIdleMetrics(true);
}
}
void MessageLoop::BindToCurrentThread() {
......
......@@ -555,9 +555,6 @@ bool MessageLoopImpl::DoIdleWork() {
if (ShouldQuitWhenIdle()) {
pump_->Quit();
} else if (task_execution_allowed_) {
if (should_record_idle_metrics_)
pending_task_queue_.ReportMetricsOnIdle();
#if defined(OS_WIN)
// On Windows we activate the high resolution timer so that the wait
// _if_ triggered by the timer happens with good resolution. If we don't
......@@ -578,9 +575,4 @@ bool MessageLoopImpl::DoIdleWork() {
return false;
}
void MessageLoopImpl::SetShouldRecordIdleMetrics(
bool should_record_idle_metrics) {
should_record_idle_metrics_ = should_record_idle_metrics;
}
} // namespace base
......@@ -96,8 +96,6 @@ class BASE_EXPORT MessageLoopImpl : public MessagePump::Delegate,
// Runs the specified PendingTask.
void RunTask(PendingTask* pending_task);
void SetShouldRecordIdleMetrics(bool should_record_idle_metrics);
// Configure various members and bind this message loop to the current thread.
void BindToCurrentThread(std::unique_ptr<MessagePump> pump);
......@@ -225,8 +223,6 @@ class BASE_EXPORT MessageLoopImpl : public MessagePump::Delegate,
std::unique_ptr<internal::ScopedSetSequenceLocalStorageMapForCurrentThread>
scoped_set_sequence_local_storage_map_for_current_thread_;
bool should_record_idle_metrics_ = false;
// Verifies that calls are made on the thread on which BindToCurrentThread()
// was invoked.
THREAD_CHECKER(bound_thread_checker_);
......
......@@ -1499,63 +1499,6 @@ TEST_P(MessageLoopTypedTest, NestableTasksAllowedManually) {
run_loop.Run();
}
#if defined(OS_MACOSX)
// This metric is a bit broken on Mac OS because CFRunLoop doesn't
// deterministically invoke MessageLoop::DoIdleWork(). This being a temporary
// diagnosis metric, we let this fly and simply not test it on Mac.
#define MAYBE_MetricsOnlyFromUILoops DISABLED_MetricsOnlyFromUILoops
#else
#define MAYBE_MetricsOnlyFromUILoops MetricsOnlyFromUILoops
#endif
TEST_P(MessageLoopTypedTest, MAYBE_MetricsOnlyFromUILoops) {
MessageLoop loop(GetParam());
const bool histograms_expected = GetParam() == MessageLoop::TYPE_UI;
HistogramTester histogram_tester;
// A delay which is expected to give enough time for the MessageLoop to go
// idle after triaging it.
TimeDelta delay_that_leads_to_idle = TimeDelta::FromMilliseconds(1);
// On some platforms testing under emulation, 1 ms is not enough. See how long
// it takes to resolve a 1ms delayed task and use 10X that for the real test.
{
TimeTicks begin_run_loop = TimeTicks::Now();
RunLoop run_loop;
loop.task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(),
delay_that_leads_to_idle);
run_loop.Run();
delay_that_leads_to_idle = 10 * (TimeTicks::Now() - begin_run_loop);
}
SCOPED_TRACE(delay_that_leads_to_idle);
// Loop that goes idle with one pending task.
RunLoop run_loop;
loop.task_runner()->PostDelayedTask(FROM_HERE, run_loop.QuitClosure(),
delay_that_leads_to_idle);
run_loop.Run();
const std::vector<Bucket> buckets = histogram_tester.GetAllSamples(
"MessageLoop.DelayedTaskQueueForUI.PendingTasksCountOnIdle");
if (histograms_expected) {
// DoIdleWork() should have triggered at least once in the second RunLoop.
// It may also have triggered in the first one if the test environment is
// fast enough. It can sometimes also trigger additional times when a system
// message (e.g. system ping) interrupts the sleep. All cases should
// nonetheless report in the "1" bucket.
EXPECT_EQ(buckets.size(), 1U);
EXPECT_EQ(buckets[0].min, 1);
EXPECT_GE(buckets[0].count, 1);
} else {
EXPECT_TRUE(buckets.empty());
}
}
INSTANTIATE_TEST_CASE_P(,
MessageLoopTypedTest,
::testing::Values(MessageLoop::TYPE_DEFAULT,
......
......@@ -17,12 +17,6 @@ PendingTaskQueue::PendingTaskQueue() = default;
PendingTaskQueue::~PendingTaskQueue() = default;
void PendingTaskQueue::ReportMetricsOnIdle() const {
UMA_HISTOGRAM_COUNTS_1M(
"MessageLoop.DelayedTaskQueueForUI.PendingTasksCountOnIdle",
delayed_tasks_.Size());
}
PendingTaskQueue::DelayedQueue::DelayedQueue() {
// The constructing sequence is not necessarily the running sequence, e.g. in
// the case of a MessageLoop created unbound.
......
......@@ -52,10 +52,6 @@ class PendingTaskQueue {
return delayed_tasks_.HasPendingHighResolutionTasks();
}
// Reports UMA metrics about its queues before the MessageLoop goes to sleep
// per being idle.
void ReportMetricsOnIdle() const;
private:
// The queue for holding tasks that should be run later and sorted by expected
// run time.
......
......@@ -50483,6 +50483,9 @@ uploading your change for review.
</histogram>
<histogram name="MessageLoop.DelayedTaskQueueForUI.PendingTasksCountOnIdle">
<obsolete>
Deprecated as of 11/2018.
</obsolete>
<owner>gab@chromium.org</owner>
<summary>
The size of the delayed task queue when the loop becomes idle on a UI
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