From 0a7d2970e5947e7a2940a32e6975817a389b0708 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 7 Jul 2020 14:13:03 +1000 Subject: [PATCH 1/4] Eliminate `rust_input`. It has a single call site and having it as a separate (higher-order!) function makes the code harder to read. --- src/librustdoc/lib.rs | 54 ++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 8e2dd77cc1155..b02880ab4d3de 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -471,7 +471,29 @@ fn main_options(options: config::Options) -> i32 { // but we can't crates the Handler ahead of time because it's not Send let diag_opts = (options.error_format, options.edition, options.debugging_options.clone()); let show_coverage = options.show_coverage; - rust_input(options, move |out| { + + // First, parse the crate and extract all relevant information. + info!("starting to run rustc"); + + // Interpret the input file as a rust source file, passing it through the + // compiler all the way through the analysis passes. The rustdoc output is + // then generated from the cleaned AST of the crate. This runs all the + // plug/cleaning passes. + let result = rustc_driver::catch_fatal_errors(move || { + let crate_name = options.crate_name.clone(); + let crate_version = options.crate_version.clone(); + let (mut krate, renderinfo, renderopts) = core::run_core(options); + + info!("finished with rustc"); + + if let Some(name) = crate_name { + krate.name = name + } + + krate.version = crate_version; + + let out = Output { krate, renderinfo, renderopts }; + if show_coverage { // if we ran coverage, bail early, we don't need to also generate docs at this point // (also we didn't load in any of the useful passes) @@ -491,36 +513,6 @@ fn main_options(options: config::Options) -> i32 { rustc_driver::EXIT_FAILURE } } - }) -} - -/// Interprets the input file as a rust source file, passing it through the -/// compiler all the way through the analysis passes. The rustdoc output is then -/// generated from the cleaned AST of the crate. -/// -/// This form of input will run all of the plug/cleaning passes -fn rust_input(options: config::Options, f: F) -> R -where - R: 'static + Send, - F: 'static + Send + FnOnce(Output) -> R, -{ - // First, parse the crate and extract all relevant information. - info!("starting to run rustc"); - - let result = rustc_driver::catch_fatal_errors(move || { - let crate_name = options.crate_name.clone(); - let crate_version = options.crate_version.clone(); - let (mut krate, renderinfo, renderopts) = core::run_core(options); - - info!("finished with rustc"); - - if let Some(name) = crate_name { - krate.name = name - } - - krate.version = crate_version; - - f(Output { krate, renderinfo, renderopts }) }); match result { From 1e8ec2db1d3d695958a8040fbb3491a4378ef7ae Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 7 Jul 2020 13:10:19 +1000 Subject: [PATCH 2/4] Add an explanatory comment to `scoped_thread`. --- src/librustc_interface/util.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index fe091e920627c..438b72b0b6176 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -102,6 +102,8 @@ impl Write for Sink { } } +/// Like a `thread::Builder::spawn` followed by a `join()`, but avoids the need +/// for `'static` bounds. #[cfg(not(parallel_compiler))] pub fn scoped_thread R + Send, R: Send>(cfg: thread::Builder, f: F) -> R { struct Ptr(*mut ()); From 4ad5de22d182578e846a6ccc69940e76a820381c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 7 Jul 2020 13:15:02 +1000 Subject: [PATCH 3/4] Tweak `spawn_thread_pool`. This makes the two versions (parallel and non-parallel) more similar to each other. --- src/librustc_interface/util.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index 438b72b0b6176..dc82219b33286 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -142,7 +142,7 @@ pub fn spawn_thread_pool R + Send, R: Send>( crate::callbacks::setup_callbacks(); - scoped_thread(cfg, || { + let main_handler = move || { rustc_ast::with_session_globals(edition, || { ty::tls::GCX_PTR.set(&Lock::new(0), || { if let Some(stderr) = stderr { @@ -151,7 +151,9 @@ pub fn spawn_thread_pool R + Send, R: Send>( f() }) }) - }) + }; + + scoped_thread(cfg, main_handler) } #[cfg(parallel_compiler)] @@ -161,12 +163,9 @@ pub fn spawn_thread_pool R + Send, R: Send>( stderr: &Option>>>, f: F, ) -> R { - use rayon::{ThreadBuilder, ThreadPool, ThreadPoolBuilder}; - - let gcx_ptr = &Lock::new(0); crate::callbacks::setup_callbacks(); - let mut config = ThreadPoolBuilder::new() + let mut config = rayon::ThreadPoolBuilder::new() .thread_name(|_| "rustc".to_string()) .acquire_thread_handler(jobserver::acquire_thread) .release_thread_handler(jobserver::release_thread) @@ -177,7 +176,7 @@ pub fn spawn_thread_pool R + Send, R: Send>( config = config.stack_size(size); } - let with_pool = move |pool: &ThreadPool| pool.install(move || f()); + let with_pool = move |pool: &rayon::ThreadPool| pool.install(move || f()); rustc_ast::with_session_globals(edition, || { rustc_ast::SESSION_GLOBALS.with(|ast_session_globals| { @@ -190,10 +189,12 @@ pub fn spawn_thread_pool R + Send, R: Send>( let main_handler = move |thread: ThreadBuilder| { rustc_ast::SESSION_GLOBALS.set(ast_session_globals, || { rustc_span::SESSION_GLOBALS.set(span_session_globals, || { - if let Some(stderr) = stderr { - io::set_panic(Some(box Sink(stderr.clone()))); - } - ty::tls::GCX_PTR.set(gcx_ptr, || thread.run()) + ty::tls::GCX_PTR.set(&Lock::new(0), || { + if let Some(stderr) = stderr { + io::set_panic(Some(box Sink(stderr.clone()))); + } + thread.run() + }) }) }) }; From bf7078615b868f7359bff58933fd5236fabe7280 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 7 Jul 2020 13:41:08 +1000 Subject: [PATCH 4/4] Change some function names. A couple of these are quite long, but they do a much better job of explaining what they do, which was non-obvious before. --- src/librustc_interface/interface.rs | 16 ++++++++-------- src/librustc_interface/util.rs | 6 +++--- src/librustdoc/core.rs | 2 +- src/librustdoc/lib.rs | 5 ++++- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/librustc_interface/interface.rs b/src/librustc_interface/interface.rs index f89be02099e8c..e50622a005379 100644 --- a/src/librustc_interface/interface.rs +++ b/src/librustc_interface/interface.rs @@ -159,10 +159,7 @@ pub struct Config { pub registry: Registry, } -pub fn run_compiler_in_existing_thread_pool( - config: Config, - f: impl FnOnce(&Compiler) -> R, -) -> R { +pub fn create_compiler_and_run(config: Config, f: impl FnOnce(&Compiler) -> R) -> R { let registry = &config.registry; let (sess, codegen_backend) = util::create_session( config.opts, @@ -204,17 +201,20 @@ pub fn run_compiler_in_existing_thread_pool( pub fn run_compiler(mut config: Config, f: impl FnOnce(&Compiler) -> R + Send) -> R { log::trace!("run_compiler"); let stderr = config.stderr.take(); - util::spawn_thread_pool( + util::setup_callbacks_and_run_in_thread_pool_with_globals( config.opts.edition, config.opts.debugging_opts.threads, &stderr, - || run_compiler_in_existing_thread_pool(config, f), + || create_compiler_and_run(config, f), ) } -pub fn default_thread_pool(edition: edition::Edition, f: impl FnOnce() -> R + Send) -> R { +pub fn setup_callbacks_and_run_in_default_thread_pool_with_globals( + edition: edition::Edition, + f: impl FnOnce() -> R + Send, +) -> R { // the 1 here is duplicating code in config.opts.debugging_opts.threads // which also defaults to 1; it ultimately doesn't matter as the default // isn't threaded, and just ignores this parameter - util::spawn_thread_pool(edition, 1, &None, f) + util::setup_callbacks_and_run_in_thread_pool_with_globals(edition, 1, &None, f) } diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index dc82219b33286..fb5f3581b6dfd 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -128,7 +128,7 @@ pub fn scoped_thread R + Send, R: Send>(cfg: thread::Builder, f: } #[cfg(not(parallel_compiler))] -pub fn spawn_thread_pool R + Send, R: Send>( +pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Send, R: Send>( edition: Edition, _threads: usize, stderr: &Option>>>, @@ -157,7 +157,7 @@ pub fn spawn_thread_pool R + Send, R: Send>( } #[cfg(parallel_compiler)] -pub fn spawn_thread_pool R + Send, R: Send>( +pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Send, R: Send>( edition: Edition, threads: usize, stderr: &Option>>>, @@ -186,7 +186,7 @@ pub fn spawn_thread_pool R + Send, R: Send>( // span_session_globals are captured and set on the new // threads. ty::tls::with_thread_locals sets up thread local // callbacks from librustc_ast. - let main_handler = move |thread: ThreadBuilder| { + let main_handler = move |thread: rayon::ThreadBuilder| { rustc_ast::SESSION_GLOBALS.set(ast_session_globals, || { rustc_span::SESSION_GLOBALS.set(span_session_globals, || { ty::tls::GCX_PTR.set(&Lock::new(0), || { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index db98ec5d0a72e..80cc5182bef32 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -376,7 +376,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt registry: rustc_driver::diagnostics_registry(), }; - interface::run_compiler_in_existing_thread_pool(config, |compiler| { + interface::create_compiler_and_run(config, |compiler| { compiler.enter(|queries| { let sess = compiler.session(); diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index b02880ab4d3de..57151e2b20002 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -437,7 +437,10 @@ fn main_args(args: &[String]) -> i32 { Ok(opts) => opts, Err(code) => return code, }; - rustc_interface::interface::default_thread_pool(options.edition, move || main_options(options)) + rustc_interface::interface::setup_callbacks_and_run_in_default_thread_pool_with_globals( + options.edition, + move || main_options(options), + ) } fn wrap_return(diag: &rustc_errors::Handler, res: Result<(), String>) -> i32 {