Allow use of "--config defaults" in bench, like GM

Allow use of "--config defaults" in bench, like GM

R=mtklein@google.com, scroggo@google.com

Committed: https://code.google.com/p/skia/source/detail?r=11298

[scroggo, 2013-09-16 18:01:47]

lgtm

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp#newcode278
bench/benchmain.cpp:278: DEFINE_string(config, "", "Run configs given.  If
empty, runs the defaults set in gConfigs.");
Maybe update the help line?

[mtklein, 2013-09-16 18:02:59]

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp#newcode278
bench/benchmain.cpp:278: DEFINE_string(config, "", "Run configs given.  If
empty, runs the defaults set in gConfigs.");
Why don't we default this to "defaults", and cut out the part about what happens
when empty?

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp#newcode335
bench/benchmain.cpp:335: } else if (0 == strcmp(FLAGS_config[i],
kDefaultsConfigStr)) {
This may seem like heresy, but I think "defaults" might be easier to read than
kDefaultsConfigStr.  If nothing else it's shorter and you don't have to cross
reference it.

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp#newcode341
bench/benchmain.cpp:341: if (configs.count() == 0 || runDefaultConfigs) {
If you change the default value of --config, I'd cut out the configs.count() ==
0 part here.  Keeps it simpler.

[borenet, 2013-09-16 18:23:50]

Uploaded patch set 2.

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp#newcode278
bench/benchmain.cpp:278: DEFINE_string(config, "", "Run configs given.  If
empty, runs the defaults set in gConfigs.");
On 2013/09/16 18:02:59, mtklein wrote:
> Why don't we default this to "defaults", and cut out the part about what
happens
> when empty?

Done.

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp#newcode335
bench/benchmain.cpp:335: } else if (0 == strcmp(FLAGS_config[i],
kDefaultsConfigStr)) {
On 2013/09/16 18:02:59, mtklein wrote:
> This may seem like heresy, but I think "defaults" might be easier to read than
> kDefaultsConfigStr.  If nothing else it's shorter and you don't have to cross
> reference it.

I kept it the same, since I added a reference in the help line.  For the record,
this is the name used in GM.

https://codereview.chromium.org/23855006/diff/1/bench/benchmain.cpp#newcode341
bench/benchmain.cpp:341: if (configs.count() == 0 || runDefaultConfigs) {
On 2013/09/16 18:02:59, mtklein wrote:
> If you change the default value of --config, I'd cut out the configs.count()
==
> 0 part here.  Keeps it simpler.

Removing that condition causes no configs to be run when I do something like,
"bench --config someNonExistantConfig", whereas now it would just run the
defaults.  Do we care?

I'm also not doing any checking for whether you've provided one of the defaults
in addition to "defaults", so, for example, running "bench --config defaults
GPU" will run the GPU benches twice.

[mtklein, 2013-09-16 19:03:56]

lgtm

[borenet, 2013-09-16 19:12:22]

Committed patchset #2 manually as r11298 (presubmit successful).