Closed
Bug 1414921
Opened 6 years ago
Closed 6 years ago
mach try fuzzy doesn't support: mozharness: --geckoProfile
Categories
(Firefox Build System :: Task Configuration, task)
Firefox Build System
Task Configuration
Tracking
(firefox61 fixed)
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: jmaher, Assigned: ahal)
References
Details
(Whiteboard: [PI:April][perf-tools])
Attachments
(2 files)
many developers want to use --geckoProfile when running talos, this is not an option right now with ./mach try fuzzy.
Reporter | ||
Comment 1•6 years ago
|
||
here is suggested syntax for geckoProfile: ./mach try -b o -p linux64 -u none -t chromez-e10s mozharness: --geckoProfile
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Updated•6 years ago
|
Whiteboard: [PI:January]
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•6 years ago
|
||
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•6 years ago
|
||
(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 hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8940264 -
Flags: review?(rwood)
Assignee | ||
Comment 6•6 years ago
|
||
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•6 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #6) ... Ok, sounds good, thanks for the update Andrew
Reporter | ||
Comment 8•6 years ago
|
||
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.
Assignee | ||
Comment 9•6 years ago
|
||
Hm, I guess there must be if it works for try syntax.. I'll look into this a little more.
Assignee | ||
Comment 10•6 years ago
|
||
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.
Updated•6 years ago
|
Whiteboard: [PI:March] → [PI:March][perf-tools]
Updated•6 years ago
|
Product: TaskCluster → Firefox Build System
Reporter | ||
Comment 12•6 years ago
|
||
we have all our tests running on taskcluster now, is this something we can finish off?
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 13•6 years ago
|
||
Yeah, I'll rebase and see how it looks on try.
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 14•6 years ago
|
||
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.
Reporter | ||
Updated•6 years ago
|
Whiteboard: [PI:March][perf-tools] → [PI:April][perf-tools]
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 17•6 years ago
|
||
mozreview-review |
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 18•6 years ago
|
||
mozreview-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+
Assignee | ||
Comment 19•6 years ago
|
||
mozreview-review-reply |
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•6 years ago
|
||
mozreview-review-reply |
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 hidden (mozreview-request) |
Comment 24•6 years ago
|
||
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
Comment 25•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0d14a11cf2ec https://hg.mozilla.org/mozilla-central/rev/bafce174742a
Status: ASSIGNED → RESOLVED
Closed: 6 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.
Description
•