Closed Bug 1289122 Opened 8 years ago Closed 8 years ago

Remove coalescing from PGO try jobs


(Firefox Build System :: Task Configuration, task)

Not set


(Not tracked)



(Reporter: mtabara, Assigned: mtabara)




(1 file, 1 obsolete file)

As per we need to unplug coalescing from PGO try jobs.
Sorry for the long delay in pushing this long-overdue patch for review.

Two questions:

1. Not sure how to test this change. Should I build (no tests) across all platforms on try?
2. I've used a regex there instead of direct removal having in mind that we might change the naming convention of the files at some point. In the current form, the patch affects PGO builds on try only. Do we want the same behavior for Opt/Debug Linux32 builds as well? (which are currently using coalescer as well per ). If so, I should get rid of that trailing "pgo" in the regex.

Thank you!
Flags: needinfo?(dustin)

I really don't care what you do to, so this seems fine :)

As for testing, you can just run `mach taskgraph -p parameters.yml target` with `parameters.yml` set up for a try job, and look at the resulting JSON (say, using `jq`).  If it generates what you want, you're all set!  You can also use 'diff' on the output of that command before and after applying your patch, if you'd like.
Comment on attachment 8774339 [details]
Bug 1289122 - Remove coalescing from PGO try jobs.
Attachment #8774339 - Flags: review?(dustin) → review+
Flags: needinfo?(dustin)

Thanks for the info on this. I played a bit with the `mach taskgraph target` command, feeding it parameters.yml from a today's earlier try push and found a small bug in the initial code: removing supersederUrl for Linux32 Opt and Debug builds. I chained supersederUrl to the routes deletion and retested. Everything looks fine in the tests.

@dustin: if this gets green light, could you please land it for me as well? I don't have S3 commit level access.
Thanks again for the help.
Comment on attachment 8774539 [details]
Bug 1289122 - bugfix to chain together the elements to be removed.

Looks good.  Do you mind squashing these two together and pushing to review again?  After that, I can click the autolander button.
Attachment #8774539 - Flags: review?(dustin) → review+
Comment on attachment 8774339 [details]
Bug 1289122 - Remove coalescing from PGO try jobs.

Review request updated; see interdiff:
Attachment #8774539 - Attachment is obsolete: true

Thanks again! Squashed and pushed. Ready to land.
Pushed by
Remove coalescing from PGO try jobs. r=dustin
Closed: 8 years ago
Resolution: --- → FIXED
In bug 1286075, I'm going to change this to not coalesce at all on try (which is, I think, the desired behavior).
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.