Commit 05399593 authored by fmeawad's avatar fmeawad Committed by Commit bot

We have noticed a clock shift when QPC was deployed. It shows as a regression on the perf bots

https://chromeperf.appspot.com/report?masters=ChromiumPerf&bots=chromium-rel-win8-dual&tests=startup.warm.dirty.blank_page%2Fwindow_display_time&rev=286928

It is not a real regression, the initial_time and initial_ticks are not properly initialized when switching to HighResolution (i.e. QPC).
This CL initializes the now_function to the HighResNowWrapper instead of setting it to RolloverProtectedNow then to the HighResNowWrapper.

By doing that, we avoid getting an incorrect initial_time and initial_ticks using the RolloverProtectedNow and avoid having to reinitialize.

BUG=158234

Review URL: https://codereview.chromium.org/446203002

Cr-Commit-Position: refs/heads/master@{#291974}
parent 07aef0e9
...@@ -135,7 +135,6 @@ void TestSuite::InitializeFromCommandLine(int argc, wchar_t** argv) { ...@@ -135,7 +135,6 @@ void TestSuite::InitializeFromCommandLine(int argc, wchar_t** argv) {
void TestSuite::PreInitialize(bool create_at_exit_manager) { void TestSuite::PreInitialize(bool create_at_exit_manager) {
#if defined(OS_WIN) #if defined(OS_WIN)
testing::GTEST_FLAG(catch_exceptions) = false; testing::GTEST_FLAG(catch_exceptions) = false;
base::TimeTicks::SetNowIsHighResNowIfSupported();
#endif #endif
base::EnableTerminationOnHeapCorruption(); base::EnableTerminationOnHeapCorruption();
#if defined(OS_LINUX) && defined(USE_AURA) #if defined(OS_LINUX) && defined(USE_AURA)
......
...@@ -639,14 +639,6 @@ class BASE_EXPORT TimeTicks { ...@@ -639,14 +639,6 @@ class BASE_EXPORT TimeTicks {
// This is only for testing. // This is only for testing.
static bool IsHighResClockWorking(); static bool IsHighResClockWorking();
// Enable high resolution time for TimeTicks::Now(). This function will
// test for the availability of a working implementation of
// QueryPerformanceCounter(). If one is not available, this function does
// nothing and the resolution of Now() remains 1ms. Otherwise, all future
// calls to TimeTicks::Now() will have the higher resolution provided by QPC.
// Returns true if high resolution time was successfully enabled.
static bool SetNowIsHighResNowIfSupported();
// Returns a time value that is NOT rollover protected. // Returns a time value that is NOT rollover protected.
static TimeTicks UnprotectedNow(); static TimeTicks UnprotectedNow();
#endif #endif
......
...@@ -359,21 +359,24 @@ bool IsBuggyAthlon(const base::CPU& cpu) { ...@@ -359,21 +359,24 @@ bool IsBuggyAthlon(const base::CPU& cpu) {
class HighResNowSingleton { class HighResNowSingleton {
public: public:
HighResNowSingleton() HighResNowSingleton()
: ticks_per_second_(0), : ticks_per_second_(0),
skew_(0) { skew_(0) {
InitializeClock();
base::CPU cpu; base::CPU cpu;
if (IsBuggyAthlon(cpu)) if (IsBuggyAthlon(cpu))
DisableHighResClock(); return;
}
bool IsUsingHighResClock() { // Synchronize the QPC clock with GetSystemTimeAsFileTime.
return ticks_per_second_ != 0.0; LARGE_INTEGER ticks_per_sec = {0};
if (!QueryPerformanceFrequency(&ticks_per_sec))
return; // QPC is not available.
ticks_per_second_ = ticks_per_sec.QuadPart;
skew_ = UnreliableNow() - ReliableNow();
} }
void DisableHighResClock() { bool IsUsingHighResClock() {
ticks_per_second_ = 0.0; return ticks_per_second_ != 0;
} }
TimeDelta Now() { TimeDelta Now() {
...@@ -408,16 +411,6 @@ class HighResNowSingleton { ...@@ -408,16 +411,6 @@ class HighResNowSingleton {
} }
private: private:
// Synchronize the QPC clock with GetSystemTimeAsFileTime.
void InitializeClock() {
LARGE_INTEGER ticks_per_sec = {0};
if (!QueryPerformanceFrequency(&ticks_per_sec))
return; // Broken, we don't guarantee this function works.
ticks_per_second_ = ticks_per_sec.QuadPart;
skew_ = UnreliableNow() - ReliableNow();
}
// Get the number of microseconds since boot in an unreliable fashion. // Get the number of microseconds since boot in an unreliable fashion.
int64 UnreliableNow() { int64 UnreliableNow() {
LARGE_INTEGER now; LARGE_INTEGER now;
...@@ -446,7 +439,6 @@ TimeDelta HighResNowWrapper() { ...@@ -446,7 +439,6 @@ TimeDelta HighResNowWrapper() {
} }
typedef TimeDelta (*NowFunction)(void); typedef TimeDelta (*NowFunction)(void);
NowFunction now_function = RolloverProtectedNow;
bool CPUReliablySupportsHighResTime() { bool CPUReliablySupportsHighResTime() {
base::CPU cpu; base::CPU cpu;
...@@ -460,6 +452,14 @@ bool CPUReliablySupportsHighResTime() { ...@@ -460,6 +452,14 @@ bool CPUReliablySupportsHighResTime() {
return true; return true;
} }
NowFunction GetNowFunction() {
if (!CPUReliablySupportsHighResTime())
return RolloverProtectedNow;
return HighResNowWrapper;
}
NowFunction now_function = GetNowFunction();
} // namespace } // namespace
// static // static
...@@ -473,16 +473,6 @@ TimeTicks::TickFunctionType TimeTicks::SetMockTickFunction( ...@@ -473,16 +473,6 @@ TimeTicks::TickFunctionType TimeTicks::SetMockTickFunction(
return old; return old;
} }
// static
bool TimeTicks::SetNowIsHighResNowIfSupported() {
if (!CPUReliablySupportsHighResTime()) {
return false;
}
now_function = HighResNowWrapper;
return true;
}
// static // static
TimeTicks TimeTicks::Now() { TimeTicks TimeTicks::Now() {
return TimeTicks() + now_function(); return TimeTicks() + now_function();
......
...@@ -158,7 +158,6 @@ int DoUninstallTasks(bool chrome_still_running) { ...@@ -158,7 +158,6 @@ int DoUninstallTasks(bool chrome_still_running) {
ChromeBrowserMainPartsWin::ChromeBrowserMainPartsWin( ChromeBrowserMainPartsWin::ChromeBrowserMainPartsWin(
const content::MainFunctionParams& parameters) const content::MainFunctionParams& parameters)
: ChromeBrowserMainParts(parameters) { : ChromeBrowserMainParts(parameters) {
base::TimeTicks::SetNowIsHighResNowIfSupported();
if (base::win::IsMetroProcess()) { if (base::win::IsMetroProcess()) {
typedef const wchar_t* (*GetMetroSwitches)(void); typedef const wchar_t* (*GetMetroSwitches)(void);
GetMetroSwitches metro_switches_proc = reinterpret_cast<GetMetroSwitches>( GetMetroSwitches metro_switches_proc = reinterpret_cast<GetMetroSwitches>(
......
...@@ -661,8 +661,6 @@ class ContentMainRunnerImpl : public ContentMainRunner { ...@@ -661,8 +661,6 @@ class ContentMainRunnerImpl : public ContentMainRunner {
MachBroker::ChildSendTaskPortToParent(); MachBroker::ChildSendTaskPortToParent();
} }
#elif defined(OS_WIN) #elif defined(OS_WIN)
base::TimeTicks::SetNowIsHighResNowIfSupported();
bool init_device_scale_factor = true; bool init_device_scale_factor = true;
if (command_line.HasSwitch(switches::kDeviceScaleFactor)) { if (command_line.HasSwitch(switches::kDeviceScaleFactor)) {
std::string scale_factor_string = command_line.GetSwitchValueASCII( std::string scale_factor_string = command_line.GetSwitchValueASCII(
......
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