Perf/decision task - #1050
Perf/decision task#1050marco-c wants to merge 7 commits into
Conversation
create_tasks rescanned every not yet submitted task each time a createTask call completed, rebuilding its set of dependencies and checking whether their futures were done. This is quadratic in the number of tasks, and runs on the main thread while holding the GIL. It also recursed once per completed batch, so a dependency chain of a few thousand tasks raised RecursionError. Now the number of pending dependencies of each task is computed once, and decremented as dependencies are created. A task is submitted as soon as its count drops to zero. As before, tasks depending on a task that failed to be created are not submitted. With createTask mocked out, creating a 20,000 task graph takes 1.9s instead of 6.2s (binary tree) and 2.0s instead of 4.6s (fan-out/fan-in).
_recurse walks every task definition when resolving task references during optimization, and again when resolving timestamps during task creation. For each single-key dictionary it iterated over all the parameter functions, building a set of the dictionary keys to compare against each of them. Now the key is looked up directly in the parameter functions. Dictionaries are also checked before lists, as they are much more common in task definitions. On a typical task definition, resolve_task_references takes 14.5us instead of 18.3us, and resolve_timestamps 20.5us instead of 23.1us.
`value_of` parsed the same few relative time strings ("1 day",
"28 days", ...) with a regex every time, although task definitions
contain several of them and are resolved multiple times. Its results
are timedeltas, which are immutable, so they are now cached.
On a typical task definition, resolve_timestamps takes 16.4us instead of
20.5us.
Graphs are immutable, but visit_postorder and visit_preorder sorted them topologically again every time they were called. The full task graph is visited once per registered verification (11 times in taskgraph alone), then again to serialize it. The target task graph is visited several times during optimization. Now the order is computed once per graph and direction, and cached like links_and_reverse_links_dict already is. Also, during optimization: - index paths are gathered by iterating over the tasks directly, as the order doesn't matter; - remove_tasks uses the cached reverse links instead of building them again. On a synthetic graph of 20,210 tasks and 40,200 edges, verifying the full task graph takes 0.34s instead of 0.73s, and optimizing it takes 1.57s instead of 2.19s.
To decide whether a task can be replaced, replace_tasks computes the latest deadline of its dependents. It resolved the deadline of every dependent for every task, although tasks such as docker images or toolchains share thousands of dependents. Now each task's deadline is resolved at most once, relative to a single `now`. Similarly, IndexSearch parsed the same deadline (and the expiration of tasks used as replacement for multiple tasks) over and over, so parsed timestamps are now cached. On a synthetic graph of 20,210 tasks where the 210 build and docker tasks are replaced, replace_tasks takes 0.17s instead of 0.24s.
The decision task rebuilt Task objects for the whole full task graph from its JSON representation, only to check that TaskGraph.from_json works. This is already covered by the TaskGraph tests, and costs time proportional to the size of the full task graph on every decision task.
Before loading a kind, the generator filtered all the tasks loaded so far to find the ones belonging to the kind's dependencies (copying them first when loading kinds in parallel). This is proportional to the number of kinds times the number of tasks, and in parallel mode it happens on the main thread, delaying the submission of newly unblocked kinds. Now loaded tasks are also grouped by kind, so the tasks of each kind dependency are looked up directly. They are still passed in the order in which kinds were loaded. With 150 kinds of 270 tasks each (40,500 tasks), each depending on three other kinds, gathering the kind dependency tasks takes 0.01s in total instead of 0.42s.
Merging this PR will improve performance by ×3.3
Performance Changes
Tip Curious why performance improved? Comment Comparing |
|
wahou |
|
@codspeedbot run the new benchmarks against the base of pr, and display the performance changes with the head of the PR, with the link to the corresponding CodSpeed compare runs page |
To let the performance wizard handle your request, please sign in to CodSpeed at codspeed.io so we can link your GitHub account, then comment again. |
|
@codspeedbot run the new benchmarks against the base of pr, and display the performance changes with the head of the PR, with the link to the corresponding CodSpeed compare runs page |
Only members of the |
|
@codspeedbot run the new benchmarks against the base of pr, and display the performance changes with the head of the PR, with the link to the corresponding CodSpeed compare runs page |
I ran the PR's benchmark suite ( Compare runs page: https://app.codspeed.io/taskcluster/taskgraph/compare/6ab631c04cedfe38851b4dfa..6ab6348ca84b5010809f92e6 New benchmarks (base → head)
Existing benchmarks, same pair of runs
The other 17 benchmarks ( How to read these numbers
I didn't change anything on the PR branch. Summary
Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot. |
|
Hey @marco-c, @JohanLorenzo, the correct compare page URL is: https://app.codspeed.io/taskcluster/taskgraph/runs/compare/6ab631c04cedfe38851b4dfa..6ab6348ca84b5010809f92e6 |
I haven't yet reviewed all the commits, I'll probably drop some of them, but I want to see if codspeed picks anything up.