Remove DepKind::CrateMetadata and pre-allocation of DepNodes#80602
Remove DepKind::CrateMetadata and pre-allocation of DepNodes#80602bors merged 3 commits intorust-lang:masterfrom
Conversation
|
r? @lcnr (rust-highfive has picked a reviewer for you, use r? to override) |
|
@rustbot label T-compiler A-incr-comp |
|
This also removes an optimization around registration of the crate metadata dependency, but I'm happy to add it back if necessary. Locally, the perf impact seemed to be small, so I think the simplification may be worth it. This also relates to logic affecting #62649. That issue has a lot of confusion around it, and this change will help make reasoning about it easier. I will follow-up with that issue at some point. |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. |
|
⌛ Trying commit a3c3735cefaf039ec7f45e46f858ba2d641c5138 with merge a9d18ad03c95138870091e989d84f332af0a0b23... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
a3c3735 to
de92295
Compare
|
I really need to automate |
de92295 to
ef71435
Compare
|
The command seems to have been lost. |
|
Awaiting bors try build completion. |
|
⌛ Trying commit ef71435e28c6743397d22957685a397b1e419da7 with merge 6da8d0299f5b2280b306e89ebfd7cb6902fcab5f... |
|
☀️ Try build successful - checks-actions |
|
Queued 6da8d0299f5b2280b306e89ebfd7cb6902fcab5f with parent 929f66a, future comparison URL. @rustbot label: +S-waiting-on-perf |
|
Finished benchmarking try commit (6da8d0299f5b2280b306e89ebfd7cb6902fcab5f): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
Perf is neutral, which is good. |
|
Interesting idea! I hope I'll be able to review this next week. |
cjgillot
left a comment
There was a problem hiding this comment.
This PR effectively replaces the creation of a dedicated DepNode for each crate by using the DepNode corresponding to the crate_hash query. Since the former behaviour based the greenness of the crate DepNode on this very crate_hash, there should be no change in the incremental behaviour.
65da797 to
0525d28
Compare
|
Addressed comments and rebased to fix a conflict. |
|
☔ The latest upstream changes (presumably #78452) made this pull request unmergeable. Please resolve the merge conflicts. |
0525d28 to
3cbe0ad
Compare
michaelwoerister
left a comment
There was a problem hiding this comment.
This is a great find, @tgnottingham! It lets us get rid of quite a bit of special casing indeed. r=me with the comment below addressed.
|
☔ The latest upstream changes (presumably #80463) made this pull request unmergeable. Please resolve the merge conflicts. |
Remove much of the special-case handling around crate metadata dependency tracking by replacing `DepKind::CrateMetadata` and the pre-allocation of corresponding `DepNodes` with on-demand invocation of the `crate_hash` query.
9bc5aa9 to
8e7cbc2
Compare
|
Thank you for the review @michaelwoerister! Addressed your comments. |
|
@bors r=michaelwoerister |
|
📌 Commit 8e7cbc2 has been approved by |
|
☀️ Test successful - checks-actions |
Remove much of the special-case handling around crate metadata
dependency tracking by replacing
DepKind::CrateMetadataand thepre-allocation of corresponding
DepNodeswith on-demand invocationof the
crate_hashquery.