Use LapTimer in rasterize_and_record micro benchmark.

Use LapTimer in rasterize_and_record micro benchmark.

Timing record or rasterize on very small layers can be problematic. The
operations are very short and timing them individually can lead to quantization.
Use a LapTimer to run the operations we're timing for at least a specified
amount of time to avoid this problem.

Moves LapTimer from cc/test to cc/debug so it's accessible outside of testing
code.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270170

[Dominik Grewe, 2014-05-12 12:14:11]

PTAL.

I moved the bitmap & canvas initialization into the loop. Without
re-initializing them each time, it seems like AnalyzeInRect() and
RasterToBitmap() take no time at all after the first run (do they early-out?).
Is there a better way?

[danakj, 2014-05-12 17:16:41]

+ernstm

https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
File cc/debug/rasterize_and_record_benchmark.cc (right):

https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark.cc:124: LapTimer timer(kWarmupRuns,
Can you move the LapTimer constant declarations down here?

https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark.cc:125:
base::TimeDelta::FromMilliseconds(kTimeLimitMillis),
And this could go inside the for loop?

https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
File cc/debug/rasterize_and_record_benchmark_impl.cc (right):

https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark_impl.cc:48: 
Can you move the LapTimer constant declarations down here?

https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
cc/debug/rasterize_and_record_benchmark_impl.cc:49: LapTimer timer(kWarmupRuns,
This could go inside he for loop?

[Dominik Grewe, 2014-05-12 17:24:23]

On 2014/05/12 17:16:41, danakj wrote:
> +ernstm
> 
>
https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
> File cc/debug/rasterize_and_record_benchmark.cc (right):
> 
>
https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
> cc/debug/rasterize_and_record_benchmark.cc:124: LapTimer timer(kWarmupRuns,
> Can you move the LapTimer constant declarations down here?
> 
>
https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
> cc/debug/rasterize_and_record_benchmark.cc:125:
> base::TimeDelta::FromMilliseconds(kTimeLimitMillis),
> And this could go inside the for loop?
> 
>
https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
> File cc/debug/rasterize_and_record_benchmark_impl.cc (right):
> 
>
https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
> cc/debug/rasterize_and_record_benchmark_impl.cc:48: 
> Can you move the LapTimer constant declarations down here?
> 
>
https://codereview.chromium.org/279183002/diff/20001/cc/debug/rasterize_and_r...
> cc/debug/rasterize_and_record_benchmark_impl.cc:49: LapTimer
timer(kWarmupRuns,
> This could go inside he for loop?

Done.

[vmpstr, 2014-05-12 18:46:08]

This makes it a little bit awkward in that we now use the lap timer as "Now"
essentially. A nicer change would be to introduce a concept of best run time
into lap timer, and a concept of running the lap timer for a specified number of
runs instead of specified number of seconds. I realize however, that it's a
possibly complicated and large change. With that, lgtm.

[Dominik Grewe, 2014-05-12 19:01:56]

On 2014/05/12 18:46:08, vmpstr wrote:
> This makes it a little bit awkward in that we now use the lap timer as "Now"
> essentially. A nicer change would be to introduce a concept of best run time
> into lap timer, and a concept of running the lap timer for a specified number
of
> runs instead of specified number of seconds. I realize however, that it's a
> possibly complicated and large change. With that, lgtm.

Do you mean having the LapTimer control both the outer and the inner loop?
The reason we're running for a specified amount of time rather than a fixed
number of iterations in the inner loop is that we don't need to run large layers
repeatedly. We're only doing it for small layers where quantization is a
problem. For large layers, we still only run one iteration of the inner loop and
actually still compute the minimum, rather than just an approximation.

[vmpstr, 2014-05-12 19:36:39]

On 2014/05/12 19:01:56, Dominik Grewe wrote:
> On 2014/05/12 18:46:08, vmpstr wrote:
> > This makes it a little bit awkward in that we now use the lap timer as "Now"
> > essentially. A nicer change would be to introduce a concept of best run time
> > into lap timer, and a concept of running the lap timer for a specified
number
> of
> > runs instead of specified number of seconds. I realize however, that it's a
> > possibly complicated and large change. With that, lgtm.
> 
> Do you mean having the LapTimer control both the outer and the inner loop?
> The reason we're running for a specified amount of time rather than a fixed
> number of iterations in the inner loop is that we don't need to run large
layers
> repeatedly. We're only doing it for small layers where quantization is a
> problem. For large layers, we still only run one iteration of the inner loop
and
> actually still compute the minimum, rather than just an approximation.

Yeah, I mean the whole repeat_count and min time is there to eliminate noise,
so if lap timer could control inner and outer loops it would be more
straightforward

I was thinking something like

lap_timer.SetTotalRunCount(repeat_count_);
do {
 ...
 lap_timer.NextLap();
} while (lap_timer.StillNeedsToRun());
min_time = lap_timer.BestRunTime();

for both inner and outer loops.

I mean it's probably not a trivial change in lap_timer to get that
functionality,
since we don't check the time after every run. That's why I'm ok with this
change
as well :)

[Dominik Grewe, 2014-05-12 20:24:06]

On 2014/05/12 19:36:39, vmpstr wrote:
> On 2014/05/12 19:01:56, Dominik Grewe wrote:
> > On 2014/05/12 18:46:08, vmpstr wrote:
> > > This makes it a little bit awkward in that we now use the lap timer as
"Now"
> > > essentially. A nicer change would be to introduce a concept of best run
time
> > > into lap timer, and a concept of running the lap timer for a specified
> number
> > of
> > > runs instead of specified number of seconds. I realize however, that it's
a
> > > possibly complicated and large change. With that, lgtm.
> > 
> > Do you mean having the LapTimer control both the outer and the inner loop?
> > The reason we're running for a specified amount of time rather than a fixed
> > number of iterations in the inner loop is that we don't need to run large
> layers
> > repeatedly. We're only doing it for small layers where quantization is a
> > problem. For large layers, we still only run one iteration of the inner loop
> and
> > actually still compute the minimum, rather than just an approximation.
> 
> Yeah, I mean the whole repeat_count and min time is there to eliminate noise,
> so if lap timer could control inner and outer loops it would be more
> straightforward
> 
> I was thinking something like
> 
> lap_timer.SetTotalRunCount(repeat_count_);
> do {
>  ...
>  lap_timer.NextLap();
> } while (lap_timer.StillNeedsToRun());
> min_time = lap_timer.BestRunTime();
> 
> for both inner and outer loops.
> 
> I mean it's probably not a trivial change in lap_timer to get that
> functionality,
> since we don't check the time after every run. That's why I'm ok with this
> change
> as well :)

Yeah, that would require a bit more work and may not be how the perf tests wanna
use it.
Anyway, thanks!

[Dominik Grewe, 2014-05-12 20:24:15]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-05-12 20:24:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominikg@chromium.org/279183002/40001

[commit-bot: I haz the power, 2014-05-13 00:38:21]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)

[commit-bot: I haz the power, 2014-05-13 00:48:35]

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

[commit-bot: I haz the power, 2014-05-13 00:48:36]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)

[Dominik Grewe, 2014-05-13 15:03:04]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-05-13 15:03:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominikg@chromium.org/279183002/80001

[commit-bot: I haz the power, 2014-05-13 19:41:01]

Change committed as 270170