Avoid creating anonymous nodes with zero or one dependency. #85337
Conversation
|
r? @varkor (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 95481bee74447fff3d637efc94e7ee0b12404ffe with merge 03d36a4a6ed12e8c2b5723d2bafbb9c71636f1b9... |
|
☀️ Try build successful - checks-actions |
|
Queued 03d36a4a6ed12e8c2b5723d2bafbb9c71636f1b9 with parent 2a245f4, future comparison URL. |
|
Finished benchmarking try commit (03d36a4a6ed12e8c2b5723d2bafbb9c71636f1b9): 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 |
rustbot
removed
the
S-waiting-on-perf
|
That's a nice idea. But it made me realize that a single |
|
@cjgillot Would you be willing to open an issue about |
| @@ -45,6 +45,7 @@ rustc_index::newtype_index! { | |||
|
|
|||
| impl DepNodeIndex { | |||
| pub const INVALID: DepNodeIndex = DepNodeIndex::MAX; | |||
| pub const DUMMY_ANON: DepNodeIndex = DepNodeIndex::from_u32(0); | |||
There was a problem hiding this comment.
Let's rename this to something more descriptive like SINGLETON_DEPENDENCYLESS_ANON_NODE.
| let current = | ||
| CurrentDepGraph::new(prev_graph_node_count, encoder, record_graph, record_stats); | ||
|
|
||
| // Instantiate an *always green* node for dependency-less anonymous queries. |
There was a problem hiding this comment.
I would not bring the fact that dependency-less nodes are always green into play here because it actually does not matter for the optimization, right?
| }; | ||
| let dep_node_index = match task_deps.len() { | ||
| 0 => { | ||
| // Dependency-less anonymous nodes can safely be replaced by a dummy node. |
There was a problem hiding this comment.
Let's provide an explanation here how this actually works. Something like:
// Because the dep-node id of anon nodes is computed from the sets of its dependencies
// we already know what the ID of this dependency-less node is going to be (i.e. equal
// to the precomputed `SINGLETON_DEPENDENCYLESS_ANON_NODE`). As a consequence we can
// skip creating a `StableHasher` and sending the node through interning.
|
@bors r+ |
|
📌 Commit b51f24f has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Anonymous nodes are only useful to encode dependencies, and cannot be replayed from one compilation session to another.
As such, anonymous nodes without dependency are always green.
Anonymous nodes with only one dependency are equivalent to this dependency.
cc #45408
cc @michaelwoerister