Skip to content

Commit

Permalink
Revert of Initialize the now_funciton to the HighResNowWrapper in cas…
Browse files Browse the repository at this point in the history
…e High Res is supported (patchset #7 of https://codereview.chromium.org/446203002/)

Reason for revert:
It causes a Canary crash

https://code.google.com/p/chromium/issues/detail?id=408354

The added static initializer causes a crash in libpeerconnetion used by WebRTC

Fix to reland: remove static initializer.

Original issue's description:
> 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
>
> Committed: https://chromium.googlesource.com/chromium/src/+/10c40c221c314e41add0a5b4df1ee7467681a430

TBR=jar@chromium.org,willchan@chromium.org,maruel@chromium.org,thakis@chromium.org,jam@chromium.org,cpu@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=158234

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

Cr-Commit-Position: refs/heads/master@{#292288}
  • Loading branch information
fmeawad authored and Commit bot committed Aug 28, 2014
1 parent 8348200 commit 3a38fb5
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 20 deletions.
1 change: 1 addition & 0 deletions base/test/test_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ void TestSuite::InitializeFromCommandLine(int argc, wchar_t** argv) {
void TestSuite::PreInitialize(bool create_at_exit_manager) {
#if defined(OS_WIN)
testing::GTEST_FLAG(catch_exceptions) = false;
base::TimeTicks::SetNowIsHighResNowIfSupported();
#endif
base::EnableTerminationOnHeapCorruption();
#if defined(OS_LINUX) && defined(USE_AURA)
Expand Down
8 changes: 8 additions & 0 deletions base/time/time.h
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,14 @@ class BASE_EXPORT TimeTicks {
// This is only for testing.
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.
static TimeTicks UnprotectedNow();
#endif
Expand Down
50 changes: 30 additions & 20 deletions base/time/time_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -373,24 +373,21 @@ bool IsBuggyAthlon(const base::CPU& cpu) {
class HighResNowSingleton {
public:
HighResNowSingleton()
: ticks_per_second_(0),
skew_(0) {
: ticks_per_second_(0),
skew_(0) {
InitializeClock();

base::CPU cpu;
if (IsBuggyAthlon(cpu))
return;

// Synchronize the QPC clock with GetSystemTimeAsFileTime.
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();
DisableHighResClock();
}

bool IsUsingHighResClock() {
return ticks_per_second_ != 0;
return ticks_per_second_ != 0.0;
}

void DisableHighResClock() {
ticks_per_second_ = 0.0;
}

TimeDelta Now() {
Expand Down Expand Up @@ -425,6 +422,16 @@ class HighResNowSingleton {
}

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.
int64 UnreliableNow() {
LARGE_INTEGER now;
Expand Down Expand Up @@ -453,6 +460,7 @@ TimeDelta HighResNowWrapper() {
}

typedef TimeDelta (*NowFunction)(void);
NowFunction now_function = RolloverProtectedNow;

bool CPUReliablySupportsHighResTime() {
base::CPU cpu;
Expand All @@ -466,14 +474,6 @@ bool CPUReliablySupportsHighResTime() {
return true;
}

NowFunction GetNowFunction() {
if (!CPUReliablySupportsHighResTime())
return RolloverProtectedNow;
return HighResNowWrapper;
}

NowFunction now_function = GetNowFunction();

} // namespace

// static
Expand All @@ -487,6 +487,16 @@ TimeTicks::TickFunctionType TimeTicks::SetMockTickFunction(
return old;
}

// static
bool TimeTicks::SetNowIsHighResNowIfSupported() {
if (!CPUReliablySupportsHighResTime()) {
return false;
}

now_function = HighResNowWrapper;
return true;
}

// static
TimeTicks TimeTicks::Now() {
return TimeTicks() + now_function();
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chrome_browser_main_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ int DoUninstallTasks(bool chrome_still_running) {
ChromeBrowserMainPartsWin::ChromeBrowserMainPartsWin(
const content::MainFunctionParams& parameters)
: ChromeBrowserMainParts(parameters) {
base::TimeTicks::SetNowIsHighResNowIfSupported();
if (base::win::IsMetroProcess()) {
typedef const wchar_t* (*GetMetroSwitches)(void);
GetMetroSwitches metro_switches_proc = reinterpret_cast<GetMetroSwitches>(
Expand Down
2 changes: 2 additions & 0 deletions content/app/content_main_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,8 @@ class ContentMainRunnerImpl : public ContentMainRunner {
MachBroker::ChildSendTaskPortToParent();
}
#elif defined(OS_WIN)
base::TimeTicks::SetNowIsHighResNowIfSupported();

bool init_device_scale_factor = true;
if (command_line.HasSwitch(switches::kDeviceScaleFactor)) {
std::string scale_factor_string = command_line.GetSwitchValueASCII(
Expand Down

0 comments on commit 3a38fb5

Please sign in to comment.