feat: generate kinds in parallel across multiple processes #765
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This still needs more testing in Gecko; I'm opening it as a draft for now to sanity check CI on the taskgraph side. |
bhearsum
force-pushed
the
push-wxroyxouptzo
branch
from
55e92c3 to
02bd9de
Compare
|
For posterity, I did try using a ThreadPoolExecutor as an alternative solution on Windows (which ought to have brought similar perf gains if using a free threaded Python). That seemed to run into concurrency issues that the multiprocess version didn't, so I didn't pursue it further. I don't think it's a bad idea to look at this again in the future, when we're already using free threaded python. |
|
Looks like this "works" on Windows this time: https://treeherder.mozilla.org/jobs?repo=try&revision=7eb3590d144a7720e1b4f4714c6429f9eb258fdf (aka: it uses the old serial generation). |
bhearsum
force-pushed
the
push-wxroyxouptzo
branch
from
02bd9de to
d9d50ca
Compare
jcristau
reviewed
Sep 9, 2025
bhearsum
force-pushed
the
push-wxroyxouptzo
branch
from
d9d50ca to
7e13a79
Compare
This is a cleaned up and slightly improved version of @ahal's original patch. Most notably, it uses `wait` to resubmit new kinds as soon as they become available (instead of waiting for all kinds in each round to be completed). This means that if a slow kind gets submitted before all other (non-downstream) kinds have been submitted, that it won't block them. In the case of Gecko, the effect of this is that the `test` kind begins to process very quickly, and all other kinds are finished processing before that has completed. Locally, this took `./mach taskgraph tasks` from 1m26s to 1m9s (measured from command start to the final "Generated xxx tasks" message. On try the results were a bit more mixed. The minimum time I observed without this patch was 140s, while the maximum was 291s (which seems to have been caused by bugbug slowness...which I'm willing to throw out). Outside of that outlier, the maximum was 146s and the mean was 143s. The minimum time I observed with this patch was 130s, while the maximum was 144s and the mean was 138s. I presume the difference in results locally vs. Try is that locally I'm on a 64-core SSD machine, and the decision tasks run on lowered powered machines on Try, so there ends up being some resource contention (I/O, I suspect, because the ProcessPoolExecutor will only run one process per CPU core) when we process kinds in parallel there. Despite this disappointing result on Try, this may still be worth taking, as `./mach taskgraph` runs twice in the critical path of many try pushes (once on a developer's machine, and again in the decision task). raw data: Over 5 runs on try I got, without this patch: 291s, 146s, 146s, 140s, 140s In each of those, there were 241s, 92s, 94s, 90s, 90s between "Loading tasks for kind test" and "Generated xxxxxx tasks for kind test" Which means we spent the following amount of time doing non-test kind things in the critical path: 50s, 54s, 52s, 50s, 50s With this patch: 130s, 141s, and 144s, 140s, 135s In each of those, there were 105s, 114s, 115s, 114s, 109s between "Loading tasks for kind test" and "Generated xxxxxx tasks for kind test" Which means we spent the following amount of time doing non-test kind things, but it was almost entirely out of the critical path: 25s, 27s, 29s, 26s, 26s
bhearsum
force-pushed
the
push-wxroyxouptzo
branch
from
7e13a79 to
9e42c6a
Compare
bhearsum
added a commit
to bhearsum/taskgraph
that referenced
this pull request
Sep 11, 2025
…ocess kind generation This was supposed to be done in taskcluster#765, but clearly I didn't push it before merging.
bhearsum
added a commit
to bhearsum/taskgraph
that referenced
this pull request
Sep 11, 2025
…ocess kind generation This was supposed to be done in taskcluster#765, but clearly I didn't push it before merging.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a slightly updated version of #738 + #744, which were backed out a few weeks ago due to not working correctly on Windows. In a conversation I had today it was suggested that we should just not support multiprocess on Windows so we can at least take the win elsewhere. This patch does that, and surely no other issues will arise...