Implement lazy decoding of DefPathTable during incremental compilation #74967

Aaron1011 · 2020-07-31T03:44:24Z

PR #75813 implemented lazy decoding of the DefPathTable from crate metadata. However, it requires decoding the entire DefPathTable when incremental compilation is active, so that we can map a decoded DefPathHash to a DefId from an arbitrary crate.

This PR adds support for lazy decoding of dependency DefPathTables when incremental compilation is active.

When we load the incremental cache and dep
graph, we need the ability to map a DefPathHash to a DefId in the
current compilation session (if the corresponding definition still
exists).

This is accomplished by storing the old DefId (that is, the DefId
from the previous compilation session) for each DefPathHash we need to
remap. Since a DefPathHash includes the owning crate, the old crate is
guaranteed to be the right one (if the definition still exists). We then
use the old DefIndex as an initial guess, which we validate by
comparing the expected and actual DefPathHashes. In most cases,
foreign crates will be completely unchanged, which means that we our
guess will be correct. If our guess is wrong, we fall back to decoding
the entire DefPathTable for the foreign crate. This still represents
an improvement over the status quo, since we can skip decoding the
entire DefPathTable for other crates (where all of our guesses were
correct).

rust-highfive · 2020-07-31T03:44:27Z

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

Aaron1011 · 2020-07-31T03:44:34Z

@bors try @rust-timer queue

rust-timer · 2020-07-31T03:44:36Z

Awaiting bors try build completion

bors · 2020-07-31T03:44:46Z

⌛ Trying commit c169390670b5400b83c79abc8662ffd76eaa321a with merge efc44d114215b797e4e501a021a02c4ab500aea0...

Aaron1011 · 2020-07-31T03:49:58Z

@bors try-

Aaron1011 · 2020-07-31T04:09:50Z

@bors try

bors · 2020-07-31T04:10:02Z

⌛ Trying commit 983d06c6ac41a906cfb178e46a21a2fe5e0a561a with merge db610e4857f4482661cb2f4a30150f12ec9a6cca...

bors · 2020-07-31T04:55:37Z

☀️ Try build successful - checks-actions, checks-azure
Build commit: db610e4857f4482661cb2f4a30150f12ec9a6cca (db610e4857f4482661cb2f4a30150f12ec9a6cca)

rust-timer · 2020-07-31T04:55:38Z

Queued db610e4857f4482661cb2f4a30150f12ec9a6cca with parent c058a8b, future comparison URL.

rust-timer · 2020-07-31T09:23:02Z

Finished benchmarking try commit (db610e4857f4482661cb2f4a30150f12ec9a6cca): 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

petrochenkov · 2020-07-31T10:04:26Z

Looks like a regression for larger crates, but win for tiny crates.
(Besides the *-doc anomaly.)

Aaron1011 · 2020-07-31T11:25:19Z

@bors try @rust-timer queue

rust-timer · 2020-07-31T11:25:21Z

Awaiting bors try build completion

bors · 2020-07-31T11:25:31Z

⌛ Trying commit c581132756dd09ad72ff2e694d63441d70f2e201 with merge 6616659d34a250401009ff4286292cf29487552b...

bors · 2020-07-31T12:05:38Z

☀️ Try build successful - checks-actions, checks-azure
Build commit: 6616659d34a250401009ff4286292cf29487552b (6616659d34a250401009ff4286292cf29487552b)

rust-timer · 2020-07-31T12:05:40Z

Queued 6616659d34a250401009ff4286292cf29487552b with parent 3a92b99, future comparison URL.

rust-timer · 2020-07-31T17:00:51Z

Finished benchmarking try commit (6616659d34a250401009ff4286292cf29487552b): 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

Aaron1011 · 2020-08-06T11:32:42Z

@bors try @rust-timer queue

rust-timer · 2020-08-06T11:32:43Z

Awaiting bors try build completion

bors · 2020-08-06T11:32:57Z

⌛ Trying commit 57f40e2c3a3717367024a93b7409ee32ea3c5052 with merge 8a163407b95486330c3d2cd8b9e49d07a35187c5...

bors · 2020-11-25T21:12:29Z

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

rust-timer · 2020-11-25T21:12:30Z

Queued bf8e95436e60effbeb46a32e17df8ab7fcb0c6ad with parent db79d2f, future comparison URL.

rust-timer · 2020-11-26T00:06:55Z

Finished benchmarking try commit (bf8e95436e60effbeb46a32e17df8ab7fcb0c6ad): 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

Aaron1011 · 2020-11-26T00:09:12Z

Regressions of up to 0.1% in non-incrememtal mode.

This is now ready for review again.

pnkfelix · 2020-12-01T10:51:57Z

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -77,6 +77,10 @@ crate struct CrateMetadata {
    raw_proc_macros: Option<&'static [ProcMacro]>,
    /// Source maps for code from the crate.
    source_map_import_info: OnceCell<Vec<ImportedSourceFile>>,
+    /// For every definition in this crate, maps its `DefPathHash` to its
+    /// `DefIndex`. See `raw_def_id_to_def_id` for more details about how


should this comment say def_path_hash_to_def_id instead?

pnkfelix · 2020-12-01T10:56:44Z

compiler/rustc_query_system/src/dep_graph/graph.rs

+        // We have just loaded a deserialized `DepNode` from the previous
+        // compilation session into the current one. If this was a foreign `DefId`,
+        // then we stored additional information in the incr comp cache when we
+        // initially created its fingerprint (see `DepNodeParams::to_fingerprint`)


super super nit: missing a period ending the sentence after the parenthesis here, making this whole paragraph look like one big run-on sentence.

pnkfelix · 2020-12-01T10:57:16Z

@bors r+

bors · 2020-12-01T10:57:17Z

📌 Commit 7a9aa4f has been approved by pnkfelix

bors · 2020-12-01T14:30:11Z

⌛ Testing commit 7a9aa4f with merge 4cbda82...

bors · 2020-12-01T16:53:13Z

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 4cbda82 to master...

rustc_metadata: Remove some dead code Follow up to rust-lang#74967

Fixes rust-lang#79661 In incremental compilation mode, we update a `DefPathHash -> DefId` mapping every time we create a `DepNode` for a foreign `DefId`. This mapping is written out to the on-disk incremental cache, and is read by the next compilation session to allow us to lazily decode `DefId`s. When we decode a `DepNode` from the current incremental cache, we need to ensure that any previously-recorded `DefPathHash -> DefId` mapping gets recorded in the new mapping that we write out. However, PR rust-lang#74967 didn't do this in all cases, leading to us being unable to decode a `DefPathHash` in certain circumstances. This PR refactors some of the code around `DepNode` deserialization to prevent this kind of mistake from happening again.

…wesleywiser Properly re-use def path hash in incremental mode Fixes rust-lang#79661 In incremental compilation mode, we update a `DefPathHash -> DefId` mapping every time we create a `DepNode` for a foreign `DefId`. This mapping is written out to the on-disk incremental cache, and is read by the next compilation session to allow us to lazily decode `DefId`s. When we decode a `DepNode` from the current incremental cache, we need to ensure that any previously-recorded `DefPathHash -> DefId` mapping gets recorded in the new mapping that we write out. However, PR rust-lang#74967 didn't do this in all cases, leading to us being unable to decode a `DefPathHash` in certain circumstances. This PR refactors some of the code around `DepNode` deserialization to prevent this kind of mistake from happening again.

rust-highfive assigned petrochenkov Jul 31, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2020

This comment has been minimized.

Sign in to view

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2020

Aaron1011 force-pushed the feature/incr-def-path-table branch from c581132 to 57f40e2 Compare August 6, 2020 11:32

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2020

Aaron1011 removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Nov 26, 2020

pnkfelix reviewed Dec 1, 2020

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 1, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 1, 2020

bors merged commit 4cbda82 into rust-lang:master Dec 1, 2020

rustbot added this to the 1.50.0 milestone Dec 1, 2020

petrochenkov mentioned this pull request Dec 2, 2020

rustc_metadata: Remove some dead code #79646

Merged

matthiaskrgr mentioned this pull request Dec 4, 2020

thread 'rustc' panicked at 'called Option::unwrap() on a None value', compiler/rustc_middle/src/ty/query/mod.rs:235:5 #79661

Closed

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 4, 2020

Rollup merge of rust-lang#79646 - petrochenkov:inclean, r=davidtwco

0fbbe94

rustc_metadata: Remove some dead code Follow up to rust-lang#74967

This was referenced Dec 5, 2020

Properly re-use def path hash in incremental mode #79721

Merged

-Z dump-dep-graph no longer prints def paths since PR #74967 #79723

Open

LeSeulArtichaut mentioned this pull request Dec 7, 2020

ICE: 'called Option::unwrap() on a None value', compiler\rustc_metadata\src\rmeta\decoder.rs:1565:75 #79783

Closed

tgnottingham mentioned this pull request Dec 11, 2020

rustc_query_system: reduce dependency graph memory usage #79589

Merged

tgnottingham mentioned this pull request Jan 4, 2021

Load all DefPathHash -> DefId mappings before dumping dep graph #80609

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement lazy decoding of DefPathTable during incremental compilation #74967

Implement lazy decoding of DefPathTable during incremental compilation #74967

Aaron1011 commented Jul 31, 2020 •

edited by jyn514

Loading

rust-highfive commented Jul 31, 2020

Aaron1011 commented Jul 31, 2020

rust-timer commented Jul 31, 2020

bors commented Jul 31, 2020

Aaron1011 commented Jul 31, 2020

This comment has been minimized.

Aaron1011 commented Jul 31, 2020

bors commented Jul 31, 2020

bors commented Jul 31, 2020

rust-timer commented Jul 31, 2020

rust-timer commented Jul 31, 2020

petrochenkov commented Jul 31, 2020

Aaron1011 commented Jul 31, 2020

rust-timer commented Jul 31, 2020

bors commented Jul 31, 2020

bors commented Jul 31, 2020

rust-timer commented Jul 31, 2020

rust-timer commented Jul 31, 2020

Aaron1011 commented Aug 6, 2020

rust-timer commented Aug 6, 2020

bors commented Aug 6, 2020

bors commented Nov 25, 2020

rust-timer commented Nov 25, 2020

rust-timer commented Nov 26, 2020

Aaron1011 commented Nov 26, 2020

pnkfelix Dec 1, 2020

pnkfelix Dec 1, 2020

pnkfelix commented Dec 1, 2020

bors commented Dec 1, 2020

bors commented Dec 1, 2020

bors commented Dec 1, 2020

Implement lazy decoding of DefPathTable during incremental compilation #74967

Implement lazy decoding of DefPathTable during incremental compilation #74967

Conversation

Aaron1011 commented Jul 31, 2020 • edited by jyn514 Loading

rust-highfive commented Jul 31, 2020

Aaron1011 commented Jul 31, 2020

rust-timer commented Jul 31, 2020

bors commented Jul 31, 2020

Aaron1011 commented Jul 31, 2020

This comment has been minimized.

Aaron1011 commented Jul 31, 2020

bors commented Jul 31, 2020

bors commented Jul 31, 2020

rust-timer commented Jul 31, 2020

rust-timer commented Jul 31, 2020

petrochenkov commented Jul 31, 2020

Aaron1011 commented Jul 31, 2020

rust-timer commented Jul 31, 2020

bors commented Jul 31, 2020

bors commented Jul 31, 2020

rust-timer commented Jul 31, 2020

rust-timer commented Jul 31, 2020

Aaron1011 commented Aug 6, 2020

rust-timer commented Aug 6, 2020

bors commented Aug 6, 2020

bors commented Nov 25, 2020

rust-timer commented Nov 25, 2020

rust-timer commented Nov 26, 2020

Aaron1011 commented Nov 26, 2020

pnkfelix Dec 1, 2020

Choose a reason for hiding this comment

pnkfelix Dec 1, 2020

Choose a reason for hiding this comment

pnkfelix commented Dec 1, 2020

bors commented Dec 1, 2020

bors commented Dec 1, 2020

bors commented Dec 1, 2020

Aaron1011 commented Jul 31, 2020 •

edited by jyn514

Loading