Rescale histogram timers.

Rescale histogram timers.

R=vogelheim@chromium.org

Committed: https://crrev.com/22421bbe9f7675e0bb99af25638841c7a390800c
Cr-Commit-Position: refs/heads/master@{#26295}

[Yang, 2015-01-26 10:32:19]

On 2015/01/26 10:32:02, Yang wrote:

Note: I won't land this before I get a response from Alexei.

[vogelheim, 2015-01-26 14:46:51]

lgtm

Assuming the '10s' thing in counters.xx is fixed. The other two comments are
just nitpicks.

https://codereview.chromium.org/875873002/diff/1/src/counters.cc
File src/counters.cc (right):

https://codereview.chromium.org/875873002/diff/1/src/counters.cc#newcode70
src/counters.cc:70: name##_ = AggregatableHistogramTimer(#caption, 0, 1000000,
50, isolate);
One more '0' please. Compile times in single digit seconds aren't unheard of, so
I think the counter should go to 10s, as it previously did.

https://codereview.chromium.org/875873002/diff/1/src/counters.h
File src/counters.h (right):

https://codereview.chromium.org/875873002/diff/1/src/counters.h#newcode364
src/counters.h:364: #define MILLISECONDS *1000
I'm not super fond of this style of macro meta programming, but I guess this is
personal preference. I think I'd prefer either plain const expressions for the
conversion factors or at least macros with parameters (that is, SECONDS(10)).

Not a big deal though, so please decide as you see fit.

https://codereview.chromium.org/875873002/diff/1/src/counters.h#newcode728
src/counters.h:728: stats_counter_count
This (and above) is a super strange indent.

I take it this is clang-format. The same occured on my recent CL in the file,
and I just reverted these (local) changes and rather ignored clang-format than
mess up the code.

Not sure if there's a policy for this.

[Yang, 2015-01-26 15:09:07]

Please take another look.

https://codereview.chromium.org/875873002/diff/1/src/counters.cc
File src/counters.cc (right):

https://codereview.chromium.org/875873002/diff/1/src/counters.cc#newcode70
src/counters.cc:70: name##_ = AggregatableHistogramTimer(#caption, 0, 1000000,
50, isolate);
On 2015/01/26 14:46:50, vogelheim wrote:
> One more '0' please. Compile times in single digit seconds aren't unheard of,
so
> I think the counter should go to 10s, as it previously did.

Done.

https://codereview.chromium.org/875873002/diff/1/src/counters.h
File src/counters.h (right):

https://codereview.chromium.org/875873002/diff/1/src/counters.h#newcode364
src/counters.h:364: #define MILLISECONDS *1000
On 2015/01/26 14:46:50, vogelheim wrote:
> I'm not super fond of this style of macro meta programming, but I guess this
is
> personal preference. I think I'd prefer either plain const expressions for the
> conversion factors or at least macros with parameters (that is, SECONDS(10)).
> 
> Not a big deal though, so please decide as you see fit.
> 

Done.

https://codereview.chromium.org/875873002/diff/1/src/counters.h#newcode728
src/counters.h:728: stats_counter_count
On 2015/01/26 14:46:50, vogelheim wrote:
> This (and above) is a super strange indent.
> 
> I take it this is clang-format. The same occured on my recent CL in the file,
> and I just reverted these (local) changes and rather ignored clang-format than
> mess up the code.
> 
> Not sure if there's a policy for this.

Done.

[Yang, 2015-01-27 14:06:35]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-01-27 14:07:01]

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

[commit-bot: I haz the power, 2015-01-27 14:08:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-27 14:08:39]

Patchset 3 (id:??) landed as
https://crrev.com/22421bbe9f7675e0bb99af25638841c7a390800c
Cr-Commit-Position: refs/heads/master@{#26295}