-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Refactor dep graph representation both in memory and on disk #60035
Conversation
let mut hasher = StableHasher::new(); | ||
|
||
for &read in task_deps.reads.iter() { | ||
let read_dep_node = self.data[read].node; | ||
task_deps.reads.hash(&mut hasher); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR changes the hashing of anon tasks to just hash the DepNodeIndex
s for dependencies instead of the full DepNode
s. This should be fine since the hash is supposed to be unique per session.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors try |
⌛ Trying commit 6c3cfa52effa2197a107ccd58a9cb3458381f4f3 with merge 7c3f54e71dcfdf264db295465d3286162256548e... |
☀️ Try build successful - checks-travis |
@rust-timer build 7c3f54e71dcfdf264db295465d3286162256548e |
Success: Queued 7c3f54e71dcfdf264db295465d3286162256548e with parent efe2f32, comparison URL. |
Finished benchmarking try commit 7c3f54e71dcfdf264db295465d3286162256548e |
@bors try |
⌛ Trying commit d2bb406e1111a227464247537793911478bbe2d7 with merge d8c21d8342bdc857fda94c3164298a1b6c465413... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☀️ Try build successful - checks-travis |
@rust-timer build d8c21d8342bdc857fda94c3164298a1b6c465413 |
Success: Queued d8c21d8342bdc857fda94c3164298a1b6c465413 with parent 8aaae42, comparison URL. |
Finished benchmarking try commit d8c21d8342bdc857fda94c3164298a1b6c465413 |
I merged These changes make the wall times pretty green (max-rss looks very green as well). I've yet to add a GC scheme so currently the dep graph file will just keep growing. |
cc @nnethercote since you wanted to get rid of the dep graph memory ;) |
@bors try |
⌛ Trying commit 7d0fe768d27b8a676ceb90bf46f4b8a21ea4496f with merge 40424a53f553d824da8e34a2881643f289130138... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
[WIP] Make dep node indices persistent between sessions This makes marking dep nodes green faster (and lock free in the case with no diagnostics). This change is split out from #60035. Unlike #60035 this makes loading the dep graph slower because it loads 2 copies of the dep graph, one immutable and one mutable. Based on #61845, #61779 and #61923.
Prerequisites from dep graph refactoring rust-lang#2 Split out from rust-lang#60035 and overlaps with rust-lang#60559.
Ping from Triage: @Zoxc is there an update on this PR? Thanks! |
I want to encourage progress on this PR because the memory savings are big. (It's also a perf win -- although instruction counts increase in most cases, wall times mostly drop.) A Firefox developer was complaining recently that compilation of Stylo within Firefox was causing OOMs because memory usage was exceeding 3.5 GiB. I did some profiling runs with Massif and DHAT, and for incremental builds the dep graph dominates the memory profiles -- most of the top entries involve the dep graph. Here are some of the dep graph records for a
|
I did manage to split part of this out as #62038, but that won't make any progress until @michaelwoerister returns. |
This PR appears to be blocked on #62038 as per the author's comment. |
Stream the dep-graph to a file instead of storing it in-memory. This is a reimplementation of rust-lang#60035. Instead of storing the dep-graph in-memory, the nodes are encoded as they come into the a temporary file as they come. At the end of a successful the compilation, this file is renamed to be the persistent dep-graph, to be decoded during the next compilation session. This two-files scheme avoids overwriting the dep-graph on unsuccessful or crashing compilations. The structure of the file is modified to be the sequence of `(DepNode, Fingerprint, EdgesVec)`. The deserialization is responsible for going to the more compressed representation. The `node_count` and `edge_count` are stored in the last 16 bytes of the file, in order to accurately reserve capacity for the vectors. At the end of the compilation, the encoder is flushed and dropped. The graph is not usable after this point: any creation of a node will ICE. I had to retrofit the debugging options, which is not really pretty.
This PR changes how we save the dep graph. Instead of storing the nodes in memory until the end of the compilation they are streamed to a file (in the background with
parallel_compiler
). This should hopefully reduce the memory usage with incremental compilation somewhat.I've yet to add code to read the dep graph back into memory for dep graph debugging functionality.
I also want to get rid of
CurrentDepGraph::node_to_node_index
, but I'm not yet sure what the best approach is for that.r? @michaelwoerister