Makes TraceTicks use lowres times on Win when highres are buggy or slow

Makes TraceTicks use lowres times on Win when highres are buggy or slow

We're doing this so that we can eventually consolidate TraceTicks and
TimeTicks and have one type of timestamp that is reliable, monotonic,
and comparable.

Also, using the high-resolution clock when it's unreliable or slow is
of questionable benefit, as it's easier to make tracing tools
accommodate a coarse timer than one that's unreliable or slow.

BUG=345845
Committed: https://crrev.com/d26a96920910558328b0d6e50af72ccde3a1e625
Cr-Commit-Position: refs/heads/master@{#345449}

[charliea (OOO until 10-5), 2015-08-21 21:13:49]

charliea@chromium.org changed reviewers:
+ fmeawad@google.com

[charliea (OOO until 10-5), 2015-08-21 21:13:49]

Hi Fadi,

Would you mind doing a pre-review of this CL?

[fmeawad, 2015-08-21 21:24:39]

fmeawad@chromium.org changed reviewers:
+ fmeawad@chromium.org

[fmeawad, 2015-08-21 21:24:40]

https://codereview.chromium.org/1284053004/diff/20001/base/time/time_win.cc
File base/time/time_win.cc (right):

https://codereview.chromium.org/1284053004/diff/20001/base/time/time_win.cc#n...
base/time/time_win.cc:451: now_function = system_trace_now_function =
&RolloverProtectedNow;
Can you explain why you still need 2 variable (now_function and
system_trace_now_function)

[charliea (OOO until 10-5), 2015-08-24 17:21:44]

On 2015/08/21 21:24:40, fmeawad wrote:
> https://codereview.chromium.org/1284053004/diff/20001/base/time/time_win.cc
> File base/time/time_win.cc (right):
> 
>
https://codereview.chromium.org/1284053004/diff/20001/base/time/time_win.cc#n...
> base/time/time_win.cc:451: now_function = system_trace_now_function =
> &RolloverProtectedNow;
> Can you explain why you still need 2 variable (now_function and
> system_trace_now_function)

You're absolutely right - I don't. Went ahead and removed
g_system_trace_now_function altogether.

[fmeawad, 2015-08-24 17:39:47]

fmeawad@chromium.org changed reviewers:
- fmeawad@google.com

[fmeawad, 2015-08-24 17:39:48]

lg 2 me

[charliea (OOO until 10-5), 2015-08-24 19:40:58]

charliea@chromium.org changed reviewers:
+ nduca@chromium.org

[charliea (OOO until 10-5), 2015-08-24 19:59:10]

charliea@chromium.org changed reviewers:
+ brianderson@chromium.org
- nduca@chromium.org

[charliea (OOO until 10-5), 2015-08-24 19:59:11]

+r:brianderson
-r:nduca
+cc:nduca

[nduca, 2015-08-24 20:00:11]

nduca@chromium.org changed reviewers:
+ nduca@chromium.org

[nduca, 2015-08-24 20:00:11]

lgtm but lets go with brian for the main review

[charliea (OOO until 10-5), 2015-08-24 20:45:31]

charliea@chromium.org changed reviewers:
+ miu@chromium.org

[charliea (OOO until 10-5), 2015-08-24 20:45:32]

+r:miu (at brianderson's suggestion)

[brianderson, 2015-08-24 20:47:13]

lgtm too

[miu, 2015-08-24 21:56:08]

lgtm

[charliea (OOO until 10-5), 2015-08-25 13:47:55]

The CQ bit was checked by charliea@chromium.org

[commit-bot: I haz the power, 2015-08-25 13:48:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1284053004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1284053004/40001

[commit-bot: I haz the power, 2015-08-25 13:56:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-25 13:56:31]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[charliea (OOO until 10-5), 2015-08-25 14:01:57]

charliea@chromium.org changed reviewers:
+ jam@chromium.org

[charliea (OOO until 10-5), 2015-08-25 14:01:57]

+R:jam for OWNERS

[jam, 2015-08-25 15:23:59]

On 2015/08/25 14:01:57, charliea wrote:
> +R:jam for OWNERS

please use an owner from base/

[charliea (OOO until 10-5), 2015-08-25 15:30:58]

charliea@chromium.org changed reviewers:
+ danakj@chromium.org
- jam@chromium.org

[charliea (OOO until 10-5), 2015-08-25 15:30:59]

+R: danakj
-R:jam

[danakj, 2015-08-25 18:37:53]

https://codereview.chromium.org/1284053004/diff/40001/base/time/time_win.cc
File base/time/time_win.cc (right):

https://codereview.chromium.org/1284053004/diff/40001/base/time/time_win.cc#n...
base/time/time_win.cc:445: NowFunction now_function;
Can you add comments saying that TraceTicks::Now() will always use the same
timer as TimeTicks::Now() and why that is okay/good/better even in the case
where the QPC is expensive/unreliable?

It would also be nice to add such comment to the CL description too.

[charliea (OOO until 10-5), 2015-08-25 20:17:07]

On 2015/08/25 18:37:53, danakj wrote:
> https://codereview.chromium.org/1284053004/diff/40001/base/time/time_win.cc
> File base/time/time_win.cc (right):
> 
>
https://codereview.chromium.org/1284053004/diff/40001/base/time/time_win.cc#n...
> base/time/time_win.cc:445: NowFunction now_function;
> Can you add comments saying that TraceTicks::Now() will always use the same
> timer as TimeTicks::Now() and why that is okay/good/better even in the case
> where the QPC is expensive/unreliable?
> 
> It would also be nice to add such comment to the CL description too.

Sorry about that - I should have given you more context (and had comments doing
the same) in the first place.

[charliea (OOO until 10-5), 2015-08-25 20:17:16]

https://codereview.chromium.org/1284053004/diff/40001/base/time/time_win.cc
File base/time/time_win.cc (right):

https://codereview.chromium.org/1284053004/diff/40001/base/time/time_win.cc#n...
base/time/time_win.cc:445: NowFunction now_function;
On 2015/08/25 18:37:53, danakj wrote:
> Can you add comments saying that TraceTicks::Now() will always use the same
> timer as TimeTicks::Now() and why that is okay/good/better even in the case
> where the QPC is expensive/unreliable?
> 
> It would also be nice to add such comment to the CL description too.

Done.

[danakj, 2015-08-25 20:24:48]

LGTM

[danakj, 2015-08-25 20:25:08]

+jbauman fyi

[charliea (OOO until 10-5), 2015-08-25 20:39:06]

The CQ bit was checked by charliea@chromium.org

[charliea (OOO until 10-5), 2015-08-25 20:39:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
brianderson@chromium.org, nduca@chromium.org, miu@chromium.org
Link to the patchset: https://codereview.chromium.org/1284053004/#ps60001
(title: "Improved the comments about why we're changing timer behavior")

[commit-bot: I haz the power, 2015-08-25 20:39:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1284053004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1284053004/60001

[commit-bot: I haz the power, 2015-08-25 22:04:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-08-25 22:04:40]

Patchset 4 (id:??) landed as
https://crrev.com/d26a96920910558328b0d6e50af72ccde3a1e625
Cr-Commit-Position: refs/heads/master@{#345449}

[Nico, 2015-08-25 22:23:49]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-08-25 22:23:50]

This broke the clang/win build:

..\..\base\time\time_win.cc(383,11) : error: unused function
'InitialSystemTraceNowFunction' [-Werror,-Wunused-function]
TimeDelta InitialSystemTraceNowFunction();
          ^
1 error generated.

Can you fix, please?

[Nico, 2015-08-25 22:26:00]

On 2015/08/25 22:23:50, Nico wrote:
> This broke the clang/win build:
> 
> ..\..\base\time\time_win.cc(383,11) : error: unused function
> 'InitialSystemTraceNowFunction' [-Werror,-Wunused-function]
> TimeDelta InitialSystemTraceNowFunction();
>           ^
> 1 error generated.
> 
> Can you fix, please?

Nevermind, got it: https://codereview.chromium.org/1312313002

[charliea (OOO until 10-5), 2015-08-26 15:38:11]

On 2015/08/25 22:26:00, Nico wrote:
> On 2015/08/25 22:23:50, Nico wrote:
> > This broke the clang/win build:
> > 
> > ..\..\base\time\time_win.cc(383,11) : error: unused function
> > 'InitialSystemTraceNowFunction' [-Werror,-Wunused-function]
> > TimeDelta InitialSystemTraceNowFunction();
> >           ^
> > 1 error generated.
> > 
> > Can you fix, please?
> 
> Nevermind, got it: https://codereview.chromium.org/1312313002

Sorry about that Nico.