Renable image benchmarking

Renable image benchmarking

- Remove --images '' to renable image benchmarking
- Add a flag to disable testing JPEG's buildTileIndex, since it also leaks memory
- Do not run images on GPU
- Do not run large interlaced images on 32 bit bots
- When buildTileIndex is not being used in the subset benches, do not use it for BRD

BUG=skia:3418
BUG=skia:4469
BUG=skia:4471
BUG=skia:4360
Committed: https://skia.googlesource.com/skia/+/860e8a67190e024b7375e52e270e6bd0a17af86e

[scroggo, 2015-10-13 20:39:44]

Trying again. I have fixed (/worked around) some memory leaks in the test, which
should fix the failing bots. (Some failures were due to API changes that have
been updated in other CLs.)

[scroggo, 2015-10-13 20:40:18]

On 2015/10/13 20:39:44, scroggo wrote:
> Trying again. I have fixed (/worked around) some memory leaks in the test,
which
> should fix the failing bots. (Some failures were due to API changes that have
> been updated in other CLs.)

This is with patch set 7 (unfortunately reitveld does not make that obvious)

[scroggo, 2015-10-14 20:02:01]

Patch set 8 disables jpeg (in SkImageDecoder buildTileIndex) by default. Maybe
that will help...?

[scroggo, 2015-10-14 20:17:16]

On 2015/10/14 20:02:01, scroggo wrote:
> Patch set 8 disables jpeg (in SkImageDecoder buildTileIndex) by default. Maybe
> that will help...?

And patch set 9 *also* disables BRD when it will use SkImageDecoder...

[scroggo, 2015-10-14 21:38:19]

scroggo@google.com changed reviewers:
+ msarett@google.com

[scroggo, 2015-10-15 14:02:44]

scroggo@google.com changed reviewers:
+ mtklein@google.com

[scroggo, 2015-10-15 14:02:45]

PTAL

[mtklein, 2015-10-15 14:19:52]

lgtm, but I'm mostly trusting nanobench.cpp changes do what you want them to do.

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.py
File tools/nanobench_flags.py (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:86: match.append('~interlaced1.png')
Just in case it's not clear, there's nothing stopping us from putting these
shared bad image lists in a third .py file that both dm_flags and
nanobench_flags import.  Not saying we have to, but if you find it handy, they
can share code / data.

[msarett, 2015-10-15 14:20:57]

lgtm

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.json
File tools/nanobench_flags.json (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.j...
tools/nanobench_flags.json:24: "--scales",
This is autogenerated?

Does this mean that there is only one non-GPU nanobench bot?

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.py
File tools/nanobench_flags.py (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:83: # the 32-bit GCE bots run out of memory in DM when
running these large images
GCE?

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:85: if 'x86' in bot and not 'x86-64' in bot:
It occurs to me that the memory leaks in SkImageDecoder were probably just as
responsible for our out of memory errors as these images.

In the future, we may want to investigate trying to test these images again.

[scroggo, 2015-10-15 14:40:47]

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.json
File tools/nanobench_flags.json (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.j...
tools/nanobench_flags.json:24: "--scales",
On 2015/10/15 14:20:57, msarett wrote:
> This is autogenerated?

Yes.

> 
> Does this mean that there is only one non-GPU nanobench bot?

Mike can probably answer more about this file, but I think this is generated
file is just a sampling of what will be seen on the bots, intended to have good
coverage. If you look at self_test() in nanobench_flags.py, it only lists 6 bots
total (easier to see in the py file, but it translates here, as well), and I'm
assuming we have more than that.

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.py
File tools/nanobench_flags.py (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:83: # the 32-bit GCE bots run out of memory in DM when
running these large images
On 2015/10/15 14:20:57, msarett wrote:
> GCE?

FWIW, this comment is copied from dm_flags.py.

Google Cloud Engine (I think?). These are our bots running in the cloud.

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:85: if 'x86' in bot and not 'x86-64' in bot:
On 2015/10/15 14:20:57, msarett wrote:
> It occurs to me that the memory leaks in SkImageDecoder were probably just as
> responsible for our out of memory errors as these images.
> 
> In the future, we may want to investigate trying to test these images again.

Seems reasonable. I'm already disabling SkImageDecoder's buildTileIndex  in
nanobench, so we could try not disabling it. I'd rather leave it in to see if we
can pass, and then try removing it. Added a FIXME.

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:86: match.append('~interlaced1.png')
On 2015/10/15 14:19:52, mtklein wrote:
> Just in case it's not clear, there's nothing stopping us from putting these
> shared bad image lists in a third .py file that both dm_flags and
> nanobench_flags import.  Not saying we have to, but if you find it handy, they
> can share code / data.

I considered doing that, although I'm not sure how much overlap there will be in
total. DM is very specific about disabling certain modes, certain files, certain
platforms. Maybe we'll need to do the same with nanobench, but until it
obviously makes things more convenient, I think this will be simpler. Added a
FIXME, anyway.

[scroggo, 2015-10-15 14:40:53]

The CQ bit was checked by scroggo@google.com

[scroggo, 2015-10-15 14:40:53]

The patchset sent to the CQ was uploaded after l-g-t-m from msarett@google.com,
mtklein@google.com
Link to the patchset: https://codereview.chromium.org/1396113002/#ps230001
(title: "Add fixmes in response to comments")

[commit-bot: I haz the power, 2015-10-15 14:41:05]

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

[commit-bot: I haz the power, 2015-10-15 14:51:31]

Committed patchset #13 (id:230001) as
https://skia.googlesource.com/skia/+/860e8a67190e024b7375e52e270e6bd0a17af86e

[msarett, 2015-10-15 14:53:17]

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.py
File tools/nanobench_flags.py (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:85: if 'x86' in bot and not 'x86-64' in bot:
On 2015/10/15 14:40:47, scroggo wrote:
> On 2015/10/15 14:20:57, msarett wrote:
> > It occurs to me that the memory leaks in SkImageDecoder were probably just
as
> > responsible for our out of memory errors as these images.
> > 
> > In the future, we may want to investigate trying to test these images again.
> 
> Seems reasonable. I'm already disabling SkImageDecoder's buildTileIndex  in
> nanobench, so we could try not disabling it. I'd rather leave it in to see if
we
> can pass, and then try removing it. Added a FIXME.

On second thought, if it's not an Android bot, it's not running the leaky code.

[mtklein, 2015-10-15 15:02:56]

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.json
File tools/nanobench_flags.json (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.j...
tools/nanobench_flags.json:24: "--scales",
On 2015/10/15 14:40:47, scroggo wrote:
> On 2015/10/15 14:20:57, msarett wrote:
> > This is autogenerated?
> 
> Yes.
> 
> > 
> > Does this mean that there is only one non-GPU nanobench bot?
> 
> Mike can probably answer more about this file, but I think this is generated
> file is just a sampling of what will be seen on the bots, intended to have
good
> coverage. If you look at self_test() in nanobench_flags.py, it only lists 6
bots
> total (easier to see in the py file, but it translates here, as well), and I'm
> assuming we have more than that.

Exactly correct.  We use enough test bots to get 100% coverage of
nanobench_flags.py, and hopefully no more (but they don't hurt).

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.py
File tools/nanobench_flags.py (right):

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:83: # the 32-bit GCE bots run out of memory in DM when
running these large images

> Google Cloud Engine (I think?).
Yep.

https://codereview.chromium.org/1396113002/diff/20002/tools/nanobench_flags.p...
tools/nanobench_flags.py:86: match.append('~interlaced1.png')
On 2015/10/15 14:40:47, scroggo wrote:
> On 2015/10/15 14:19:52, mtklein wrote:
> > Just in case it's not clear, there's nothing stopping us from putting these
> > shared bad image lists in a third .py file that both dm_flags and
> > nanobench_flags import.  Not saying we have to, but if you find it handy,
they
> > can share code / data.
> 
> I considered doing that, although I'm not sure how much overlap there will be
in
> total. DM is very specific about disabling certain modes, certain files,
certain
> platforms. Maybe we'll need to do the same with nanobench, but until it
> obviously makes things more convenient, I think this will be simpler. Added a
> FIXME, anyway.

sgtm