Need a clear way to do a "is there a performance effect?" try push

RESOLVED FIXED in Firefox 68

Status

enhancement
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: bzbarsky, Assigned: ahal)

Tracking

Trunk
mozilla68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

There's no good way to do this right now that I can see. The options seem to be:

  1. trychooser webpage and syntax. "-t all" doesn't run various other performance suites (e.g raptor), and the whole thing is deprecated anyway.

  2. "mach try fuzzy". No obvious sane way to select all the "performance tests" in this interface.

  3. "mach try chooser". Has a simple way to select all the performance tests but not a way to do a --rebuild from the UI, which is needed to get useful information out of perf tests. It looks like one can do a --rebuild from the command-line, but that's not very discoverable unless you carefully read "mach help try chooser".

Ideally there would be a simple alias or built-in preset that one can use to just run our performance tests on a try push.

This could be solved via bug 1513951, which I'm planning to tackle within the next few weeks. We could make a built-in preset that would be runnable with:

$ ./mach try --preset perf

or something similar.

Depends on: 1513951

The new "perf" preset is close, but still requires the user to know about the --rebuild option to get anything like useful data.

Summary: Need a clear way to do a ""is there a performance effect?" try push → Need a clear way to do a "is there a performance effect?" try push

We could bake --rebuild 5 into the preset. Originally that's what my patch did, but we ended up removing it during review phase because:

  1. It would be impossible to remove that --rebuild in case people didn't want it
  2. A general desire to default to fewer tasks

If you think the upside is more compelling than those downsides, I'm definitely not opposed to adding it back in.

Why would one ever want to do a "perf" push with rebuild < 5? Would one ever get any useful data from such a thing? (In my limited experience, no.)

If we did have a --rebuild 5 in the preset, could one override it with a larger rebuild value? I've personally found that anything less than 10 doesn't really work in terms of getting useful data. :(

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #4)

Why would one ever want to do a "perf" push with rebuild < 5? Would one ever get any useful data from such a thing? (In my limited experience, no.)

If we did have a --rebuild 5 in the preset, could one override it with a larger rebuild value? I've personally found that anything less than 10 doesn't really work in terms of getting useful data. :(

I agree with bz here. I've also found that it's basically worthless doing "perf" pushes in isolation with the expectation of comparing to m-c: you also need to explicitly push a baseline to try with the appropriate --rebuild.

Assignee: nobody → ahal
Severity: normal → enhancement
Status: NEW → ASSIGNED

Will that still allow to override with a bigger --rebuild number at push time if desired?

Flags: needinfo?(ahal)

Yes, or a smaller one. There just isn't a way to remove it completely (and 2 is the minimum that argument will accept).

Flags: needinfo?(ahal)

That seems fine. I can't think of any use cases for doing this with --rebuild 1.

There is "I just changed the harness and need to check the tests are not busted", but I'll also note that --rebuild 1 is not clear as to what it does: does it do 2 tasks (build, rebuild), or 1? In fact the question is still true for any value given to the option. Also, those tasks are not builds. Can we just change this option name?

(In reply to Mike Hommey [:glandium] from comment #10)

There is "I just changed the harness and need to check the tests are not busted",

I'll note that there is the sample-suite preset that may address this use case (though it runs all suites, not just performance ones).

Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/21ef29c73ddf
Add '--rebuild=5' to the perf preset, DONTBUILD, r=davehunt
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.