1103995 - Make it possible to limit which mozbench benchmarks run on each platform

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Dan Minor [:dminor]]

Mozbench is a cross-browser benchmark suite. For more information, see [1] and [2]. The mozbench repository is located at [3] and instructions on setting it up and running the benchmarks can be found in the readme.md file at that location.

At the moment benchmarks are either enabled or disabled for each platform through the 'enabled' field in benchmarks.json.

I think we should change the enabled field from boolean to be a list of platforms on which to run the benchmarks.

Some examples of what I have in mind:

'enabled' : ['all']  # run on all platforms
'enabled' : ['linux', 'android']  # only run on linux and android
'enabled' : []  # disabled

We can get the platform on desktop from mozinfo.os. We can override this on b2g and android when the appropriate command line arguments are present.

[1] https://wiki.mozilla.org/Game_Benchmark_Automation
[2] https://wiki.mozilla.org/Auto-tools/Projects/Mozbench
[3] https://github.com/dminor/mozbench

Comment 1

[Ricardo Castro]

As discussed in Bug 1103035 I'll start by progressing this bug.

Comment 2

[Ricardo Castro]

Hi Dan,

Just to clarify a little bit. 

The idea is to run mozbench as we usually do, but when we do, detect either the OS or the arguments '--use-b2g' or '--use-android'. If they are not present in that list mozbench won't run, running otherwise.

Comment 3

[Dan Minor [:dminor]]

Right now we check benchmarks.json for an enabled flag for each benchmark. If it is false, we skip that benchmark. I was thinking we'd change that from a boolean to be a list of platforms for which the benchmark is enabled.

We could initialize a variable to hold the platform:
platform = mozinfo.os
if args.use_b2g:
  platform = 'b2g'
elif args.use_android:
  platform = 'android'

And then in the loop over the defined benchmarks we could change the enabled check to be something like:
if not ('all' in benchmark.enabled or platform in benchmark.enabled):
  continue

Comment 4

[Ricardo Castro]

Exactly. I was also thinking along those lines. 

Also, I was thinking about Bug 1103035 and comparing against other browsers. We  have specific browsers for specific platforms. Would something like a config file be useful? If so, could this information be added to that config file? That config file would have for each platform what browser/path to browser executable to run against. That 'config file' can be the 'benchmarks.json' but with more information.

Sorry for the rant and I'm might be going a little off topic, but I was thinking about this.

Comment 5

[Dan Minor [:dminor]]

(In reply to Ricardo Castro from comment #4)
> Exactly. I was also thinking along those lines. 
> 
> Also, I was thinking about Bug 1103035 and comparing against other browsers.
> We  have specific browsers for specific platforms. Would something like a
> config file be useful? If so, could this information be added to that config
> file? That config file would have for each platform what browser/path to
> browser executable to run against. That 'config file' can be the
> 'benchmarks.json' but with more information.
> 
> Sorry for the rant and I'm might be going a little off topic, but I was
> thinking about this.

Yeah, I suppose that could be something like for each platform have a list of browser name, path, and which runner to use for it. I'm not opposed to that idea, but why don't we do that as a separate bug after we see if those other browsers actually end up being interesting :)

People might ask for IE and Safari at some point as well, and this idea would help avoid having 20 different command line switches for each browser.

Comment 6

[Ricardo Castro]

Exactly. I was not thinking specific to this bug but maybe having a bug to track that.  Bug 1103035 already has some demand for that. I was just throwing it out in the open.

Comment 7

[Dan Minor [:dminor]]

(In reply to Ricardo Castro from comment #6)
> Exactly. I was not thinking specific to this bug but maybe having a bug to
> track that.  Bug 1103035 already has some demand for that. I was just
> throwing it out in the open.

It's a good idea. Would you like to file a bug for this, or should I?

Comment 8

[Ricardo Castro]

I'll take care of it.

Comment 9

[Ricardo Castro]

Attachment: Pull request.

Comment 10

[Dan Minor [:dminor]]

Comment on attachment 8534650 [details] [review]
Pull request.

Thanks!