State: Initialize counters with kAvgIteration in constructor #1652
Conversation
Previously, `counters` was updated in `PauseTiming()` with `counters[name] += Counter(measurement, kAvgIteration)`. The first `counters[name]` call inserts a counter with no flags. There is no `operator+=` for `Counter`, so the insertion is done by converting the `Counter` to a `double`, then constructing a `Counter` to insert from the `double`, which drops the flags. Pre-insert the `Counter` with the correct flags, then only update `Counter::value`. Introduced in 1c64a36 ([perf-counters] Fix pause/resume (google#1643)).
Perf counters are now divided by iterations, so dividing again in the test is wrong.
|
I'm not sure why most of the sanitizer tests are failing. The history on my branch is getting out of control. This should be squashed-and-merged when it's ready. |
they're passing at head, i'm not sure why your change would break them.. only thing i can think (as it's coming from __assert) is that you're referencing
it will be.. always do the squash when i merge :) |
|
Can you trigger the presubmits on HEAD? I wonder if something changed with the sanitizer setup. |
yeah they're failing for changes to documents |
|
Thanks for fixing this. I forgot how |
3 participants
Previously,
counterswas updated inPauseTiming()withcounters[name] += Counter(measurement, kAvgIteration).The first
counters[name]call inserts a counter with no flags.There is no
operator+=forCounter, so the insertion is doneby converting the
Counterto adouble, then constructing aCounterto insert from thedouble, which drops the flags.Pre-insert the
Counterwith the correct flags, then onlyupdate
Counter::value.Introduced in 1c64a36 ([perf-counters] Fix pause/resume (#1643)).