rustc_query_system: reduce dependency graph memory usage

[tgnottingham]

This change implements, at a high level, two space optimizations to the dependency graph.

The first optimization is sharing graph data with the previous dependency graph. Whenever we intern a node, we know whether that node is new (not in the previous graph) or not, and if not, the color of the node in the previous graph.

Red and green nodes have their DepNode present in the previous graph, so for that piece of node data, we can just store the index of the node in the previous graph rather than duplicate the DepNode. Green nodes additionally have the the same result Fingerprint, so we can avoid duplicating that too. Finally, we distinguish between "light" and "dark" green nodes, where the latter are nodes that were marked green because all of their dependencies were marked green. These nodes can additionally share edges with the previous graph, because we know that their set of dependencies is the same (technically, light green and red nodes can have the same dependencies too, but we don't try to figure out whether or not that's the case).

Also, some effort is made to pack data tightly, and to avoid storing DepNodes as map keys more than once.

The second optimization is storing edges in a more compact representation, as in the SerializedDepGraph, that is, in a single vector, rather than one EdgesVec per node. An EdgesVec is a SmallVec with an inline buffer for 8 elements. Each EdgesVec is, at minimum, 40 bytes, and has a per-node overhead of up to 40 bytes. In the ideal case of exactly 8 edges, then 32 bytes are used for edges, and the overhead is 8 bytes. But most of the time, the overhead is higher.

In contrast, using a single vector to store all edges, and having each node specify its start and end elements as 4 byte indices into the vector has a constant overhead of 8 bytes--the best case scenario for the per-node EdgesVec approach.

The downside of this approach is that EdgesVecs built up during query execution have to be copied into the vector, whereas before, we could just take ownership over them. However, we mostly make up for this because the single vector representation enables a more efficient implementation of DepGraph::serialize.

[rust-highfive]

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

[tgnottingham]

@rustbot label T-compiler A-incr-comp I-compilemem I-compiletime

[tgnottingham]

By the way, reviewing this is probably best done commit-by-commit, starting with the comments and structs near DepNodeData.

[lcnr]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit 07d6913867813e544e465a7a86664da43b2f855d with merge 2dcb181386ce78b9432c26d19dfe249ade022c5a...

[bors]

☀️ Try build successful - checks-actions
Build commit: 2dcb181386ce78b9432c26d19dfe249ade022c5a (2dcb181386ce78b9432c26d19dfe249ade022c5a)

[rust-timer]

Queued 2dcb181386ce78b9432c26d19dfe249ade022c5a with parent c4926d0, future comparison URL.

[rust-timer]

Finished benchmarking try commit (2dcb181386ce78b9432c26d19dfe249ade022c5a): 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 rollup- to bors.

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 modify labels: +S-waiting-on-review -S-waiting-on-perf

[lcnr]

This looks quite good to me, especially the 10% rss improvements on some real world benchmarks. I don't know enough about the internals of the query system to review this myself.

r? @nikomatsakis for review or reassignment

[jyn514]

cc also @cjgillot and @nnethercote

[wesleywiser]

cc @rust-lang/wg-incr-comp

[tgnottingham]

For the perf results, it's helpful to look at incr-full and incr-patched/unchanged results separately using the check boxes, as this change affects them differently.

Non-incremental builds shouldn't be affected much, so you can probably safely untick the full box and ignore the bootstrap timings (I assume the bootstrap benchmark doesn't use incremental).

I'm very happy with the results. You can see that incr-full takes a small hit to instruction count (but ignore the unused-warnings change -- that regularly varies by ~%1 in my experience), but I'm working on a change to improve this. I'll probably leave it for a separate PR, since this one is big enough already.

[tgnottingham]

Rebased to rename EdgesIndex to EdgeIndex.

[nnethercote]

This is a rare example of a change where the cycles and wall-time improvements are significantly larger than the instruction counts improvements. Nice work!

[tgnottingham]

Thanks, wouldn't have known where to begin without Massif! 🙏

[nikomatsakis]

Apologies for the delay! I'll try to take a look soon!

[rust-timer]

Awaiting bors try build completion.

[bors]

⌛ Trying commit 03eb75f with merge 9efae6f05396b03e469da1db40b7eedb501e0697...

[bors]

☀️ Try build successful - checks-actions
Build commit: 9efae6f05396b03e469da1db40b7eedb501e0697 (9efae6f05396b03e469da1db40b7eedb501e0697)

[rust-timer]

Queued 9efae6f05396b03e469da1db40b7eedb501e0697 with parent 969b42d, future comparison URL.

@rustbot label: +S-waiting-on-perf

[rust-timer]

Finished benchmarking try commit (9efae6f05396b03e469da1db40b7eedb501e0697): 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 rollup- to bors.

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 label: +S-waiting-on-review -S-waiting-on-perf

[bjorn3]

Except for a single 2.6% regression for coercions-debug, this doesn't have any big regressions. incr-unchanged runs have huge wins of up to 2.4%.

[michaelwoerister]

also added a change to fix a race condition.

@tgnottingham Can you elaborate on what the exact problem was there?

[michaelwoerister]

OK, I see now. Yes, it can't hurt to keep things locked throughout. Both methods are supposed to be called in situations where the dep-graph has already settled down and nothing new is added, but let's not rely on that invariant to be upheld.

[michaelwoerister]

Let's see if I have r+ rights currently: @bors r+

[bors]

@michaelwoerister: 🔑 Insufficient privileges: Not in reviewers

[michaelwoerister]

Nope... @nikomatsakis do you want to give the r+?

[lqd]

@bors r=michaelwoerister

[bors]

📌 Commit 03eb75f has been approved by michaelwoerister

[bors]

⌛ Testing commit 03eb75f with merge 49b3151...

[bors]

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing 49b3151 to master...