From d61605cef866008a19e33eb596df1d76ff1571f3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 09:03:58 +1000 Subject: [PATCH 1/5] Don't call `self.parse()` in `Compiler::crate_name()` unless necessary. --- src/librustc_interface/queries.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index ed50dadb60099..996e9fae0db56 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -126,17 +126,18 @@ impl Compiler { pub fn crate_name(&self) -> Result<&Query> { self.queries.crate_name.compute(|| { - let parse_result = self.parse()?; - let krate = parse_result.peek(); - let result = match self.crate_name { + Ok(match self.crate_name { Some(ref crate_name) => crate_name.clone(), - None => rustc_codegen_utils::link::find_crate_name( - Some(self.session()), - &krate.attrs, - &self.input - ), - }; - Ok(result) + None => { + let parse_result = self.parse()?; + let krate = parse_result.peek(); + rustc_codegen_utils::link::find_crate_name( + Some(self.session()), + &krate.attrs, + &self.input + ) + } + }) }) } From cd0c21b0e5b68e29c63c3c98db9147cb0e5a3bc8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 16:24:01 +1000 Subject: [PATCH 2/5] Remove `lower_to_hir()` call from `prepare_output()`. It's a false dependency. The result isn't used and there are no relevant side-effects. --- src/librustc_interface/queries.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 996e9fae0db56..c281bc5360704 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -195,7 +195,6 @@ impl Compiler { pub fn prepare_outputs(&self) -> Result<&Query> { self.queries.prepare_outputs.compute(|| { - self.lower_to_hir()?; let krate = self.expansion()?; let krate = krate.peek(); let crate_name = self.crate_name()?; From c6117d949922a382d7b1ba11a523ea9aeafcabd4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 11:25:16 +1000 Subject: [PATCH 3/5] Merge `Compiler::{ongoing_codegen,link}`. They each have two call sites, and the sequencing is almost identical in both cases. --- src/librustc_driver/lib.rs | 7 +------ src/librustc_interface/queries.rs | 33 +++++++++++-------------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 9a6a12e261c10..320b5b88a1d3d 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -383,17 +383,12 @@ pub fn run_compiler( mem::drop(compiler.expansion()?.take()); } - compiler.ongoing_codegen()?; - - // Drop GlobalCtxt after starting codegen to free memory - mem::drop(compiler.global_ctxt()?.take()); + compiler.codegen_and_link()?; if sess.opts.debugging_opts.print_type_sizes { sess.code_stats.borrow().print_type_sizes(); } - compiler.link()?; - if sess.opts.debugging_opts.perf_stats { sess.print_perf_stats(); } diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index c281bc5360704..5f8935dc0c256 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -83,8 +83,7 @@ pub(crate) struct Queries { codegen_channel: Query<(Steal>>, Steal>>)>, global_ctxt: Query, - ongoing_codegen: Query>, - link: Query<()>, + codegen_and_link: Query<()>, } impl Compiler { @@ -230,14 +229,14 @@ impl Compiler { }) } - pub fn ongoing_codegen(&self) -> Result<&Query>> { - self.queries.ongoing_codegen.compute(|| { + pub fn codegen_and_link(&self) -> Result<&Query<()>> { + self.queries.codegen_and_link.compute(|| { let rx = self.codegen_channel()?.peek().1.steal(); let outputs = self.prepare_outputs()?; - self.global_ctxt()?.peek_mut().enter(|tcx| { + let ongoing_codegen = self.global_ctxt()?.peek_mut().enter(|tcx| { tcx.analysis(LOCAL_CRATE).ok(); - // Don't do code generation if there were any errors + // Don't do code generation if there were any errors. self.session().compile_status()?; Ok(passes::start_codegen( @@ -246,21 +245,16 @@ impl Compiler { rx, &*outputs.peek() )) - }) - }) - } - - pub fn link(&self) -> Result<&Query<()>> { - self.queries.link.compute(|| { - let sess = self.session(); + })?; - let ongoing_codegen = self.ongoing_codegen()?.take(); + // Drop GlobalCtxt after starting codegen to free memory. + mem::drop(self.global_ctxt()?.take()); self.codegen_backend().join_codegen_and_link( ongoing_codegen, - sess, + self.session(), &*self.dep_graph()?.peek(), - &*self.prepare_outputs()?.peek(), + &*outputs.peek(), ).map_err(|_| ErrorReported)?; Ok(()) @@ -281,11 +275,6 @@ impl Compiler { // Drop AST after creating GlobalCtxt to free memory mem::drop(self.expansion()?.take()); - self.ongoing_codegen()?; - - // Drop GlobalCtxt after starting codegen to free memory - mem::drop(self.global_ctxt()?.take()); - - self.link().map(|_| ()) + self.codegen_and_link().map(|_| ()) } } From df952caf669ea62fad8029c965b7e0c3385d21cf Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 16:53:34 +1000 Subject: [PATCH 4/5] Move call site of `dep_graph_future()`. `Compiler::register_plugins()` calls `passes::register_plugins()`, which calls `Compiler::dep_graph_future()`. This is the only way in which a `passes` function calls a `Compiler` function. This commit moves the `dep_graph_future()` call out of `passes::register_plugins()` and into `Compiler::register_plugins()`, which is a more sensible spot for it. This will delay the loading of the dep graph slightly -- from the middle of plugin registration to the end of plugin registration -- but plugin registration is fast enough (especially compared to expansion) that the impact should be neglible. --- src/librustc_interface/passes.rs | 4 ---- src/librustc_interface/queries.rs | 14 +++++++++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 24b44964e4fd2..200da05c57561 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -223,7 +223,6 @@ pub struct PluginInfo { } pub fn register_plugins<'a>( - compiler: &Compiler, sess: &'a Session, cstore: &'a CStore, mut krate: ast::Crate, @@ -261,9 +260,6 @@ pub fn register_plugins<'a>( }); } - // If necessary, compute the dependency graph (in the background). - compiler.dep_graph_future().ok(); - time(sess, "recursion limit", || { middle::recursion_limit::update_limits(sess, &krate); }); diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 5f8935dc0c256..e8aa0e1b4533e 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -113,13 +113,21 @@ impl Compiler { let crate_name = self.crate_name()?.peek().clone(); let krate = self.parse()?.take(); - passes::register_plugins( - self, + let result = passes::register_plugins( self.session(), self.cstore(), krate, &crate_name, - ) + ); + + // Compute the dependency graph (in the background). We want to do + // this as early as possible, to give the DepGraph maximum time to + // load before dep_graph() is called, but it also can't happen + // until after rustc_incremental::prepare_session_directory() is + // called, which happens within passes::register_plugins(). + self.dep_graph_future().ok(); + + result }) } From 92a0cbd3c30ce26498739c8f18cba6ab0cd4e7f8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 30 Aug 2019 17:27:35 +1000 Subject: [PATCH 5/5] Remove `Compiler::compile()`. `Compiler::compile()` is different to all the other `Compiler` methods because it lacks a `Queries` entry. It only has one call site, which is in a test that doesn't need its specific characteristics. This patch removes `Compile::compile()` and replaces the call to it in the test with a call to `Compile::codegen_and_link()`, which is similar enough for the test's purposes. --- src/librustc_interface/queries.rs | 23 +++---------------- src/test/run-make-fulldeps/issue-19371/foo.rs | 2 +- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index e8aa0e1b4533e..ca56c04bebb68 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -2,17 +2,17 @@ use crate::interface::{Compiler, Result}; use crate::passes::{self, BoxedResolver, ExpansionResult, BoxedGlobalCtxt, PluginInfo}; use rustc_incremental::DepGraphFuture; -use rustc::session::config::{OutputFilenames, OutputType}; +use rustc::session::config::OutputFilenames; use rustc::util::common::{time, ErrorReported}; use rustc::hir; use rustc::hir::def_id::LOCAL_CRATE; use rustc::ty::steal::Steal; use rustc::dep_graph::DepGraph; +use std::any::Any; use std::cell::{Ref, RefMut, RefCell}; +use std::mem; use std::rc::Rc; use std::sync::mpsc; -use std::any::Any; -use std::mem; use syntax::{self, ast}; /// Represent the result of a query. @@ -268,21 +268,4 @@ impl Compiler { Ok(()) }) } - - pub fn compile(&self) -> Result<()> { - self.prepare_outputs()?; - - if self.session().opts.output_types.contains_key(&OutputType::DepInfo) - && self.session().opts.output_types.len() == 1 - { - return Ok(()) - } - - self.global_ctxt()?; - - // Drop AST after creating GlobalCtxt to free memory - mem::drop(self.expansion()?.take()); - - self.codegen_and_link().map(|_| ()) - } } diff --git a/src/test/run-make-fulldeps/issue-19371/foo.rs b/src/test/run-make-fulldeps/issue-19371/foo.rs index afc92638fda97..118205671e5d8 100644 --- a/src/test/run-make-fulldeps/issue-19371/foo.rs +++ b/src/test/run-make-fulldeps/issue-19371/foo.rs @@ -62,6 +62,6 @@ fn compile(code: String, output: PathBuf, sysroot: PathBuf) { }; interface::run_compiler(config, |compiler| { - compiler.compile().ok(); + compiler.codegen_and_link().ok(); }); }