1414921 - mach try fuzzy doesn't support: mozharness: --geckoProfile

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

many developers want to use --geckoProfile when running talos, this is not an option right now with ./mach try fuzzy.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

here is suggested syntax for geckoProfile:
./mach try -b o -p linux64 -u none -t chromez-e10s mozharness: --geckoProfile

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

Review commit: https://reviewboard.mozilla.org/r/207448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207448/

Comment 3

[Andrew Halberstadt [:ahal]]

This was a WIP I had. I ran into a bump (can't remember what anymore) and stopped working on it, then got distracted by the holidays. Rob, if you want to push on this feel free to take it away from me. Otherwise I'll try and find some time to finish this up at some point this month.

Comment 4

[Robert Wood [:rwood]]

(In reply to Andrew Halberstadt [:ahal] from comment #3)
> This was a WIP I had. I ran into a bump (can't remember what anymore) and
> stopped working on it, then got distracted by the holidays. Rob, if you want
> to push on this feel free to take it away from me. Otherwise I'll try and
> find some time to finish this up at some point this month.

Ok thanks Andrew, if I get to it before you I'll let you know!

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8940264 [details]
Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207448/diff/1-2/

Comment 6

[Andrew Halberstadt [:ahal]]

The attached patch works, but the reason I unflagged you for review was that I realized it's only going to be possible for talos tasks that are running with taskcluster natively (so OSX only atm). With this patch, if you pass in --geckoProfile and schedule a linux or windows talos task, the decision task will fail.

I could probably get linux and windows to be a no-op with a bit extra work, but it'll still be confusing to developers ("I passed in --geckoProfile but I don't see any profiles!"). So I'd recommend blocking until talos is completely off buildbot.

Comment 7

[Robert Wood [:rwood]]

(In reply to Andrew Halberstadt [:ahal] from comment #6)
...
Ok, sounds good, thanks for the update Andrew

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

at best we are looking at the end of February- I always think we need a full month after that before we can really call it done as often we are wanting to profile on a regression that landed a week or 2 ago, etc.

Is there no way to do this for linux/windows on buildbot?  If not, then we should plan on doing this in March.

Comment 9

[Andrew Halberstadt [:ahal]]

Hm, I guess there must be if it works for try syntax.. I'll look into this a little more.

Comment 10

[Andrew Halberstadt [:ahal]]

For buildbot, mozharness directly parses the try syntax (which I guess it gets via buildbot proprties or something):
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/talos.py#222

I don't have any way of setting that from the try_task_config.json morph templates (and even if I did, I still might recommend punting just due to how hacky it is ;)). So, I think we can either revisit in March, or land what we have even though it'll only work for OSX atm.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

lets revisit in March

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

we have all our tests running on taskcluster now, is this something we can finish off?

Comment 13

[Andrew Halberstadt [:ahal]]

Yeah, I'll rebase and see how it looks on try.

Comment 14

[Andrew Halberstadt [:ahal]]

Looks like it's working now (the decision task passes), but I'm hitting another issue later on in the talos task. Shouldn't take too long to sort out.

Comment 15

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1414921 - [taskgraph] Convert test_morph.py to the pytest format,

I was going to add a new test to this file and realized there was a lot of
duplication happening. Since I find it a lot easier to abstract code using
pytest's fixtures than it is to store instance variables and methods on a
unittest.TestCase class, I decided to convert this file to the pytest format
first.

Besides, I believe pytest's format makes the test a lot easier to read and
understand.

Review commit: https://reviewboard.mozilla.org/r/233772/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/233772/

Comment 16

[Andrew Halberstadt [:ahal]]

Comment on attachment 8940264 [details]
Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207448/diff/2-3/

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8965046 [details]
Bug 1414921 - [taskgraph] Convert test_morph.py to the pytest format,

https://reviewboard.mozilla.org/r/233772/#review239702

I'm not familiar with pytest, but this is nicely readable!

Comment 18

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8940264 [details]
Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

https://reviewboard.mozilla.org/r/207448/#review239692

Some minor suggestions here.  I'm starting to get the feeling that JSON-e is not the right tool here!  But this works for now..

::: commit-message-defcc:6
(Diff revision 3)
> +Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|, r?dustin
> +
> +Enables |./mach try fuzzy --geckoProfile|. This template only applies to talos
> +tasks. I'm not crazy about the name of the command line arg (would prefer
> +--talos-profile), but this is what try syntax uses, so probably best to stay
> +consistent.

Perhaps support both?  Perhaps with `argparse.SUPPRESS` for the `--geckoProfile` option?

::: tools/tryselect/templates.py:134
(Diff revision 3)
>          return {
>              'rebuild': rebuild,
>          }
>  
>  
> +class Profile(Template):

Should this maybe be named TalosProfile?

Comment 19

[Andrew Halberstadt [:ahal]]

Comment on attachment 8940264 [details]
Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

https://reviewboard.mozilla.org/r/207448/#review239692

Yeah, this is a bit hairy and will get worse as we want to do more complex things.

I have a proposal to move some of the complexity out of the json-e templates themselves over in bug 1451370. But maybe we should be looking to replace json-e with something else altogether. Please comment over there if you have any ideas :).

> Perhaps support both?  Perhaps with `argparse.SUPPRESS` for the `--geckoProfile` option?

Good idea!

Comment 20

[Andrew Halberstadt [:ahal]]

Comment on attachment 8965046 [details]
Bug 1414921 - [taskgraph] Convert test_morph.py to the pytest format,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/233772/diff/1-2/

Comment 21

[Andrew Halberstadt [:ahal]]

Comment on attachment 8940264 [details]
Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207448/diff/3-4/

Comment 22

[Andrew Halberstadt [:ahal]]

Comment on attachment 8940264 [details]
Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

https://reviewboard.mozilla.org/r/207448/#review239692

> Should this maybe be named TalosProfile?

Fixed and also renamed the template file on the taskgraph end.

Comment 23

[Andrew Halberstadt [:ahal]]

Comment on attachment 8940264 [details]
Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207448/diff/4-5/

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d14a11cf2ec
[taskgraph] Convert test_morph.py to the pytest format, r=dustin
https://hg.mozilla.org/integration/autoland/rev/bafce174742a
Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|, r=dustin

Comment 25

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0d14a11cf2ec
https://hg.mozilla.org/mozilla-central/rev/bafce174742a