cc: Only report calc_draw_props_count.

cc: Only report calc_draw_props_time.

The perf dashboard is configured to monitor every trace output by the
cc_perftests. Here, we were outputting two traces that represent the exact
same thing. This only causes more work for the perf sheriffs. So I'm cutting it
down to one.

Incidentally, this eliminates a bug where the dashboard thought that a higher
count was a regression.

This also improves the precision by supporting decimals.

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

[tonyg, 2013-11-01 21:39:50]

I'd be happy to keep count over time if you prefer that. But my goal here is to
just report one or the other in order to cut down on # of alerts received by the
sheriff.

[enne (OOO), 2013-11-01 21:46:12]

On 2013/11/01 21:39:50, tonyg wrote:
> I'd be happy to keep count over time if you prefer that. But my goal here is
to
> just report one or the other in order to cut down on # of alerts received by
the
> sheriff.

Yeah, I'd prefer to keep count.  (1) seems much more informative than (2).

(1)
https://chromeperf.appspot.com/report?rev=231895&masters=ChromiumPerf&bots=li...

(2)
https://chromeperf.appspot.com/report?rev=231895&masters=ChromiumPerf&bots=li...

We do this in a lot of cc perftests, should we change those too?

[tonyg, 2013-11-01 22:06:50]

> Yeah, I'd prefer to keep count.  (1) seems much more informative than (2).

Great catch! You are totally right.

> We do this in a lot of cc perftests, should we change those too?

I changed everywhere I could find. I think there are a lot fewer occurrences
since Dana's refactor.

[danakj, 2013-11-01 22:09:26]

On Fri, Nov 1, 2013 at 5:46 PM, <enne@chromium.org> wrote:

> On 2013/11/01 21:39:50, tonyg wrote:
>
>> I'd be happy to keep count over time if you prefer that. But my goal here
>> is
>>
> to
>
>> just report one or the other in order to cut down on # of alerts received
>> by
>>
> the
>
>> sheriff.
>>
>
> Yeah, I'd prefer to keep count.  (1) seems much more informative than (2).
>

I actually prefer ms/run over count. The amount of time the tests run each
time is variable but changes from run to run, and the count does not
include this in it's result, whereas the ms/run does.

A runs/s would handle this, but a straight-up count doesn't.


>
> (1)
> https://chromeperf.appspot.**com/report?rev=231895&masters=**
> ChromiumPerf&bots=linux-**release&tests=cc_perftests%**
>
2Fcalc_draw_props_count&**checked=touch_region_heavy<https://chromeperf.appspot.com/report?rev=231895&masters=ChromiumPerf&bots=linux-release&tests=cc_perftests%2Fcalc_draw_props_count&checked=touch_region_heavy>
>
> (2)
> https://chromeperf.appspot.**com/report?rev=231895&masters=**
> ChromiumPerf&bots=linux-**release&tests=cc_perftests%**
>
2Fcalc_draw_props_time&**checked=touch_region_heavy<https://chromeperf.appspot.com/report?rev=231895&masters=ChromiumPerf&bots=linux-release&tests=cc_perftests%2Fcalc_draw_props_time&checked=touch_region_heavy>
>
> We do this in a lot of cc perftests, should we change those too?
>
>
https://codereview.chromium.**org/56573002/<https://codereview.chromium.org/5...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tonyg, 2013-11-01 22:12:16]

On Fri, Nov 1, 2013 at 3:09 PM, Dana Jansens <danakj@chromium.org> wrote:

> On Fri, Nov 1, 2013 at 5:46 PM, <enne@chromium.org> wrote:
>
>> On 2013/11/01 21:39:50, tonyg wrote:
>>
>>> I'd be happy to keep count over time if you prefer that. But my goal
>>> here is
>>>
>> to
>>
>>> just report one or the other in order to cut down on # of alerts
>>> received by
>>>
>> the
>>
>>> sheriff.
>>>
>>
>> Yeah, I'd prefer to keep count.  (1) seems much more informative than (2).
>>
>
> I actually prefer ms/run over count. The amount of time the tests run each
> time is variable but changes from run to run, and the count does not
> include this in it's result, whereas the ms/run does.
>
> A runs/s would handle this, but a straight-up count doesn't.
>

I'd be happy to update this patch to runs/s, but will wait for enne to
confirm before making the change.


>
>
>>
>> (1)
>> https://chromeperf.appspot.**com/report?rev=231895&masters=**
>> ChromiumPerf&bots=linux-**release&tests=cc_perftests%**
>>
2Fcalc_draw_props_count&**checked=touch_region_heavy<https://chromeperf.appspot.com/report?rev=231895&masters=ChromiumPerf&bots=linux-release&tests=cc_perftests%2Fcalc_draw_props_count&checked=touch_region_heavy>
>>
>> (2)
>> https://chromeperf.appspot.**com/report?rev=231895&masters=**
>> ChromiumPerf&bots=linux-**release&tests=cc_perftests%**
>>
2Fcalc_draw_props_time&**checked=touch_region_heavy<https://chromeperf.appspot.com/report?rev=231895&masters=ChromiumPerf&bots=linux-release&tests=cc_perftests%2Fcalc_draw_props_time&checked=touch_region_heavy>
>>
>> We do this in a lot of cc perftests, should we change those too?
>>
>>
https://codereview.chromium.**org/56573002/<https://codereview.chromium.org/5...
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[enne (OOO), 2013-11-01 22:17:37]

I don't feel as strongly as danakj does about what number gets reported here. 
My only concern is that the ms/run graph looks like it's snapped to integers and
it seems like we'd be losing a lot of precision by switching to it.  If the
ms/run graph could have that precision, then I'd be happy with it.

[danakj, 2013-11-01 22:23:28]

On Fri, Nov 1, 2013 at 6:17 PM, <enne@chromium.org> wrote:

> I don't feel as strongly as danakj does about what number gets reported
> here.
> My only concern is that the ms/run graph looks like it's snapped to
> integers and
> it seems like we'd be losing a lot of precision by switching to it.  If the
> ms/run graph could have that precision, then I'd be happy with it.
>

Ya, I agree that graph looks super bogus. I think runs/s should be better
and would still account for the time the test actually ran.


>
>
https://codereview.chromium.**org/56573002/<https://codereview.chromium.org/5...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[enne (OOO), 2013-11-04 19:08:40]

On 2013/11/01 22:23:28, danakj wrote:
> On Fri, Nov 1, 2013 at 6:17 PM, <mailto:enne@chromium.org> wrote:
> 
> > I don't feel as strongly as danakj does about what number gets reported
> > here.
> > My only concern is that the ms/run graph looks like it's snapped to
> > integers and
> > it seems like we'd be losing a lot of precision by switching to it.  If the
> > ms/run graph could have that precision, then I'd be happy with it.
> 
> Ya, I agree that graph looks super bogus. I think runs/s should be better
> and would still account for the time the test actually ran.

danakj mentioned that using ops/count might be off by (2 + time-per-run) / 2,
since this test runs until it gets over 2 seconds and then reports an integer
number of runs.  I think since time per run seems to be 15ms for Android, this
error is ~1%.  Maybe that's not a big deal, but it does seem a little sloppy. 
What do you think?

I think the reason the ms/run graph is bad is that PerfTest::PrintResult takes a
size_t and has no float version, so the float gets truncated.  Fixing that would
let you go back to patch set 1 and not have this extra error.

[tonyg, 2013-11-04 23:12:44]

> I think the reason the ms/run graph is bad is that PerfTest::PrintResult takes
a
> size_t and has no float version, so the float gets truncated.  Fixing that
would
> let you go back to patch set 1 and not have this extra error.

That sgtm. Dana?

[tonyg, 2013-11-07 23:39:40]

enne, ptal

[enne (OOO), 2013-11-07 23:49:12]

lgtm

[commit-bot: I haz the power, 2013-11-07 23:56:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tonyg@chromium.org/56573002/160001

[commit-bot: I haz the power, 2013-11-08 05:31:49]

Change committed as 233775