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

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dminor, Assigned: mcc.ricardo, Mentored)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment)

41 bytes, text/x-github-pull-request
dminor
: review+
Details | Review | Splinter Review
(Reporter)

Description

4 years ago
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
(Assignee)

Comment 1

4 years ago
As discussed in Bug 1103035 I'll start by progressing this bug.
Assignee: nobody → mcc.ricardo
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
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.
Flags: needinfo?(dminor)
(Reporter)

Comment 3

4 years ago
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
Flags: needinfo?(dminor)
(Assignee)

Comment 4

4 years ago
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.
Flags: needinfo?(dminor)
(Reporter)

Comment 5

4 years ago
(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.
Flags: needinfo?(dminor)
(Assignee)

Comment 6

4 years ago
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.
(Reporter)

Comment 7

4 years ago
(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?
(Assignee)

Comment 8

4 years ago
I'll take care of it.
(Assignee)

Comment 9

4 years ago
Created attachment 8534650 [details] [review]
Pull request.
Attachment #8534650 - Flags: review?(dminor)
(Reporter)

Comment 10

4 years ago
Comment on attachment 8534650 [details] [review]
Pull request.

Thanks!
Attachment #8534650 - Flags: review?(dminor) → review+
(Reporter)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.