Commit 050feaa7 authored by Gabriel Charette's avatar Gabriel Charette

Revert "[MessageLoop] Disable hi-res timers when not sleeping"

This reverts commit 62973d1b.

Reason for revert: results in more churn in
base::Time::ActivateHighResolutionTimer()
without much benefits (decreased 97th and 98th percentile of
Windows.HighResolutionTimerUsage from 100% to 97% and 99%
respectively). See crbug.com/863938

Original change's description:
> [MessageLoop] Disable hi-res timers when not sleeping
> 
> Time::ActivateHighResolutionTimer(bool activating) is a per-thread vote
> for a system-wide side-effect. For a given thread, hi-res timers are
> only useful when going to sleep (if it has pending hi-res tasks).
> 
> Deactivating a thread's vote while it's active will prevent other
> threads on the system which do not have hi-res requirements from
> being forced to use hi-res timers in that period.
> 
> Bug: 854237
> Change-Id: I1393e184cac6c9321d13b92b6077a38c62b1f590
> Reviewed-on: https://chromium-review.googlesource.com/1107110
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: kylechar <kylechar@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574259}

TBR=danakj@chromium.org,gab@chromium.org,kylechar@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 854237, 863938
Change-Id: I7c26646cac8548ae7b02c90e045bc857ae890ce7
Reviewed-on: https://chromium-review.googlesource.com/1140753Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576157}
parent cabb24c1
......@@ -192,8 +192,7 @@ MessageLoop::~MessageLoop() {
#if defined(OS_WIN)
if (in_high_res_mode_)
Time::ActivateHighResolutionTimer(false);
#endif // defined (OS_WIN)
#endif
// Clean up any unprocessed tasks, but take care: deleting a task could
// result in the addition of more tasks (e.g., via DeleteSoon). We set a
// limit on the number of times we will allow a deleted task to generate more
......@@ -491,18 +490,6 @@ bool MessageLoop::DoWork() {
if (!task_execution_allowed_)
return false;
#if defined(OS_WIN)
// Raising timer frequency has a system-wide effect. Ensure this thread's vote
// to raise the frequency is only active while it is sleeping (with pending
// hi-res tasks) to avoid unnecessarily keeping the entire system in a high
// frequency mode while this thread isn't sleeping (even if it has pending
// hi-res delayed tasks). Ref. https://crbug.com/854237#c3
if (in_high_res_mode_) {
in_high_res_mode_ = false;
Time::ActivateHighResolutionTimer(false);
}
#endif // defined (OS_WIN)
// Execute oldest task.
while (incoming_task_queue_->triage_tasks().HasTasks()) {
if (!scheduled_wakeup_.next_run_time.is_null()) {
......@@ -673,16 +660,15 @@ bool MessageLoop::DoIdleWork() {
// for some tasks.
need_high_res_timers =
incoming_task_queue_->HasPendingHighResolutionTasks();
#endif // defined (OS_WIN)
#endif
}
#if defined(OS_WIN)
if (need_high_res_timers) {
DCHECK(!in_high_res_mode_);
in_high_res_mode_ = true;
Time::ActivateHighResolutionTimer(true);
if (in_high_res_mode_ != need_high_res_timers) {
in_high_res_mode_ = need_high_res_timers;
Time::ActivateHighResolutionTimer(in_high_res_mode_);
}
#endif // defined (OS_WIN)
#endif
// When we return we will do a kernel wait for more tasks.
return false;
......
......@@ -271,8 +271,8 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate,
const Type type_;
#if defined(OS_WIN)
// Tracks if this MessageLoop has requested system-wide high resolution timers
// for its sleep period. Used to downvote that request when the sleep is over.
// Tracks if we have requested high resolution timers. Its only use is to
// turn off the high resolution timer upon loop destruction.
bool in_high_res_mode_ = false;
#endif
......
......@@ -21,10 +21,8 @@
#include "base/posix/eintr_wrapper.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/synchronization/atomic_flag.h"
#include "base/synchronization/waitable_event.h"
#include "base/task_scheduler/task_scheduler.h"
#include "base/test/bind_test_util.h"
#include "base/test/gtest_util.h"
#include "base/test/test_simple_task_runner.h"
#include "base/test/test_timeouts.h"
......@@ -1877,71 +1875,40 @@ TEST_P(MessageLoopTest, WaitForIO) {
}
TEST_P(MessageLoopTest, HighResolutionTimer) {
Thread verification_thread("verification thread");
verification_thread.StartAndWaitForTesting();
MessageLoop message_loop;
Time::EnableHighResolutionTimer(true);
constexpr TimeDelta kFastTimer = TimeDelta::FromMilliseconds(31);
constexpr TimeDelta kFastTimer = TimeDelta::FromMilliseconds(5);
constexpr TimeDelta kSlowTimer = TimeDelta::FromMilliseconds(100);
// Since MessageLoop disables its vote to activate high-resolution timers the
// instant it's active, the high-resolution timer's activation can only be
// tested async. Try to make this as reliable as possible by verifying in the
// middle of the MessageLoop's sleep period (giving plenty of time before for
// it to establish the high-res timer and plenty of time after for it not to
// wakeup while verifying).
constexpr TimeDelta kVerificationDelay = kFastTimer / 2;
{
// Post a fast task to enable the high resolution timers.
RunLoop run_loop;
message_loop.task_runner()->PostDelayedTask(
FROM_HERE, BindLambdaForTesting([&]() {
// The loop deactivates high resolution timers the instant it's
// active.
EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
run_loop.Quit();
}),
FROM_HERE,
BindOnce(
[](RunLoop* run_loop) {
EXPECT_TRUE(Time::IsHighResolutionTimerInUse());
run_loop->QuitWhenIdle();
},
&run_loop),
kFastTimer);
// Ensures the verification runs (that thread could theoretically never be
// scheduled -- skipping the test).
AtomicFlag verification_ran;
verification_thread.task_runner()->PostDelayedTask(
FROM_HERE, BindLambdaForTesting([&]() {
EXPECT_TRUE(Time::IsHighResolutionTimerInUse());
verification_ran.Set();
}),
kVerificationDelay);
run_loop.Run();
ASSERT_TRUE(verification_ran.IsSet());
}
EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
{
// Check that a slow task does not trigger the high resolution logic.
RunLoop run_loop;
message_loop.task_runner()->PostDelayedTask(
FROM_HERE, BindLambdaForTesting([&]() {
// The loop deactivates high resolution timers the instant it's
// active.
EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
run_loop.Quit();
}),
FROM_HERE,
BindOnce(
[](RunLoop* run_loop) {
EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
run_loop->QuitWhenIdle();
},
&run_loop),
kSlowTimer);
AtomicFlag verification_ran;
verification_thread.task_runner()->PostDelayedTask(
FROM_HERE, BindLambdaForTesting([&]() {
EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
verification_ran.Set();
}),
kVerificationDelay);
run_loop.Run();
ASSERT_TRUE(verification_ran.IsSet());
}
Time::EnableHighResolutionTimer(false);
Time::ResetHighResolutionTimerUsage();
......
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