Closed Bug 1414921 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(firefox61 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: jmaher, Assigned: ahal)

References

(Blocks 1 open bug)

Details

(Whiteboard: [PI:April][perf-tools])

Attachments

(2 files)

Bug 1414921 - Add --geckoProfile to capture profiles from talos tasks to |mach try fuzzy|,
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1414921 - [taskgraph] Convert test_morph.py to the pytest format,
59 bytes, text/x-review-board-request
dustin
: review+
Details 
many developers want to use --geckoProfile when running talos, this is not an option right now with ./mach try fuzzy.
here is suggested syntax for geckoProfile:
./mach try -b o -p linux64 -u none -t chromez-e10s mozharness: --geckoProfile
Blocks: 1425103
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Whiteboard: [PI:January]
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.
(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!
Attachment #8940264 - Flags: review?(rwood)
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.
(In reply to Andrew Halberstadt [:ahal] from comment #6)
...
Ok, sounds good, thanks for the update Andrew
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.
Hm, I guess there must be if it works for try syntax.. I'll look into this a little more.
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.
lets revisit in March
Whiteboard: [PI:January] → [PI:March]
Whiteboard: [PI:March] → [PI:March][perf-tools]
Product: TaskCluster → Firefox Build System
we have all our tests running on taskcluster now, is this something we can finish off?
Flags: needinfo?(ahalberstadt)
Yeah, I'll rebase and see how it looks on try.
Flags: needinfo?(ahalberstadt)
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.
Whiteboard: [PI:March][perf-tools] → [PI:April][perf-tools]
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!
Attachment #8965046 - Flags: review?(dustin) → review+
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?
Attachment #8940264 - Flags: review?(dustin) → review+
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 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.
Pushed by ahalberstadt@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/0d14a11cf2ec
https://hg.mozilla.org/mozilla-central/rev/bafce174742a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.