From cf13d9143d2a907120682fe3d6e59249a21cb0ca Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Mon, 10 Oct 2022 10:50:14 +1100
Subject: [PATCH 1/2] Clarify `run_in_thread_pool_with_globals`.

- Make the structure of the two variants more similar.
- Add some comments.
- Move various conditional `use` items inside the function that uses
  them.
- Inline some closures.
---
 compiler/rustc_driver/src/lib.rs     |  1 +
 compiler/rustc_interface/src/util.rs | 89 ++++++++++++++--------------
 2 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/compiler/rustc_driver/src/lib.rs b/compiler/rustc_driver/src/lib.rs
index f268d50e96e87..7edbb6f757ce1 100644
--- a/compiler/rustc_driver/src/lib.rs
+++ b/compiler/rustc_driver/src/lib.rs
@@ -190,6 +190,7 @@ impl<'a, 'b> RunCompiler<'a, 'b> {
         run_compiler(self.at_args, self.callbacks, self.file_loader, self.make_codegen_backend)
     }
 }
+
 fn run_compiler(
     at_args: &[String],
     callbacks: &mut (dyn Callbacks + Send),
diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs
index 6d0fffc152c3c..519b8a7fc7c37 100644
--- a/compiler/rustc_interface/src/util.rs
+++ b/compiler/rustc_interface/src/util.rs
@@ -3,14 +3,8 @@ use libloading::Library;
 use rustc_ast as ast;
 use rustc_codegen_ssa::traits::CodegenBackend;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-#[cfg(parallel_compiler)]
-use rustc_data_structures::jobserver;
 use rustc_errors::registry::Registry;
-#[cfg(parallel_compiler)]
-use rustc_middle::ty::tls;
 use rustc_parse::validate_attr;
-#[cfg(parallel_compiler)]
-use rustc_query_impl::{QueryContext, QueryCtxt};
 use rustc_session as session;
 use rustc_session::config::CheckCfg;
 use rustc_session::config::{self, CrateType};
@@ -25,8 +19,6 @@ use rustc_span::symbol::{sym, Symbol};
 use std::env;
 use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};
 use std::mem;
-#[cfg(not(parallel_compiler))]
-use std::panic;
 use std::path::{Path, PathBuf};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::OnceLock;
@@ -135,13 +127,20 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
     _threads: usize,
     f: F,
 ) -> R {
-    // The thread pool is a single thread in the non-parallel compiler.
-    thread::scope(|s| {
-        let mut builder = thread::Builder::new().name("rustc".to_string());
-        if let Some(size) = get_stack_size() {
-            builder = builder.stack_size(size);
-        }
+    // The "thread pool" is a single spawned thread in the non-parallel
+    // compiler. We run on a spawned thread instead of the main thread (a) to
+    // provide control over the stack size, and (b) to increase similarity with
+    // the parallel compiler, in particular to ensure there is no accidental
+    // sharing of data between the main thread and the compilation thread
+    // (which might cause problems for the parallel compiler).
+    let mut builder = thread::Builder::new().name("rustc".to_string());
+    if let Some(size) = get_stack_size() {
+        builder = builder.stack_size(size);
+    }
 
+    // We build the session globals and run `f` on the spawned thread, because
+    // `SessionGlobals` does not impl `Send` in the non-parallel compiler.
+    thread::scope(|s| {
         // `unwrap` is ok here because `spawn_scoped` only panics if the thread
         // name contains null bytes.
         let r = builder
@@ -151,55 +150,57 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
 
         match r {
             Ok(v) => v,
-            Err(e) => panic::resume_unwind(e),
+            Err(e) => std::panic::resume_unwind(e),
         }
     })
 }
 
-/// Creates a new thread and forwards information in thread locals to it.
-/// The new thread runs the deadlock handler.
-/// Must only be called when a deadlock is about to happen.
-#[cfg(parallel_compiler)]
-unsafe fn handle_deadlock() {
-    let registry = rustc_rayon_core::Registry::current();
-
-    let query_map = tls::with(|tcx| {
-        QueryCtxt::from_tcx(tcx)
-            .try_collect_active_jobs()
-            .expect("active jobs shouldn't be locked in deadlock handler")
-    });
-    thread::spawn(move || rustc_query_impl::deadlock(query_map, &registry));
-}
-
 #[cfg(parallel_compiler)]
 pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
     edition: Edition,
     threads: usize,
     f: F,
 ) -> R {
-    let mut config = rayon::ThreadPoolBuilder::new()
+    use rustc_data_structures::jobserver;
+    use rustc_middle::ty::tls;
+    use rustc_query_impl::{deadlock, QueryContext, QueryCtxt};
+
+    let mut builder = rayon::ThreadPoolBuilder::new()
         .thread_name(|_| "rustc".to_string())
         .acquire_thread_handler(jobserver::acquire_thread)
         .release_thread_handler(jobserver::release_thread)
         .num_threads(threads)
-        .deadlock_handler(|| unsafe { handle_deadlock() });
-
+        .deadlock_handler(|| {
+            // On deadlock, creates a new thread and forwards information in thread
+            // locals to it. The new thread runs the deadlock handler.
+            let query_map = tls::with(|tcx| {
+                QueryCtxt::from_tcx(tcx)
+                    .try_collect_active_jobs()
+                    .expect("active jobs shouldn't be locked in deadlock handler")
+            });
+            let registry = rustc_rayon_core::Registry::current();
+            thread::spawn(move || deadlock(query_map, &registry));
+        });
     if let Some(size) = get_stack_size() {
-        config = config.stack_size(size);
+        builder = builder.stack_size(size);
     }
 
-    let with_pool = move |pool: &rayon::ThreadPool| pool.install(f);
-
+    // We create the session globals on the main thread, then create the thread
+    // pool. Upon creation, each worker thread created gets a copy of the
+    // session globals in TLS. This is possible because `SessionGlobals` impls
+    // `Send` in the parallel compiler.
     rustc_span::create_session_globals_then(edition, || {
         rustc_span::with_session_globals(|session_globals| {
-            // The main handler runs for each Rayon worker thread and sets up
-            // the thread local rustc uses. `session_globals` is captured and set
-            // on the new threads.
-            let main_handler = move |thread: rayon::ThreadBuilder| {
-                rustc_span::set_session_globals_then(session_globals, || thread.run())
-            };
-
-            config.build_scoped(main_handler, with_pool).unwrap()
+            builder
+                .build_scoped(
+                    // Initialize each new worker thread when created.
+                    move |thread: rayon::ThreadBuilder| {
+                        rustc_span::set_session_globals_then(session_globals, || thread.run())
+                    },
+                    // Run `f` on the first thread in the thread pool.
+                    move |pool: &rayon::ThreadPool| pool.install(f),
+                )
+                .unwrap()
         })
     })
 }

From 5d716fd0e96c4408ad13e5e82df364fb1e984540 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Tue, 11 Oct 2022 11:43:25 +1100
Subject: [PATCH 2/2] Add a comment to `Compiler`.

It took me a while to work this out.
---
 compiler/rustc_interface/src/interface.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs
index a3bf7cde9ff71..89aaa0b95e41b 100644
--- a/compiler/rustc_interface/src/interface.rs
+++ b/compiler/rustc_interface/src/interface.rs
@@ -25,7 +25,10 @@ use std::result;
 
 pub type Result<T> = result::Result<T, ErrorGuaranteed>;
 
-/// Represents a compiler session.
+/// Represents a compiler session. Note that every `Compiler` contains a
+/// `Session`, but `Compiler` also contains some things that cannot be in
+/// `Session`, due to `Session` being in a crate that has many fewer
+/// dependencies than this crate.
 ///
 /// Can be used to run `rustc_interface` queries.
 /// Created by passing [`Config`] to [`run_compiler`].