Auto merge of #102769 - nnethercote:rustdoc-startup, r=jyn514

Clean up rustdoc startup

Startup is pretty hairy, in both rustdoc and rustc. The first commit here improves the rustdoc situation quite a bit. The remaining commits are smaller but also help.

Best reviewed one commit at a time.

r? `@jyn514`

compiler/rustc_interface/src/interface.rs

@@ -275,58 +275,6 @@ pub struct Config {
     pub registry: Registry,
 }
 
-pub fn create_compiler_and_run<R>(config: Config, f: impl FnOnce(&Compiler) -> R) -> R {
-    crate::callbacks::setup_callbacks();
-
-    let registry = &config.registry;
-    let (mut sess, codegen_backend) = util::create_session(
-        config.opts,
-        config.crate_cfg,
-        config.crate_check_cfg,
-        config.file_loader,
-        config.input_path.clone(),
-        config.lint_caps,
-        config.make_codegen_backend,
-        registry.clone(),
-    );
-
-    if let Some(parse_sess_created) = config.parse_sess_created {
-        parse_sess_created(
-            &mut Lrc::get_mut(&mut sess)
-                .expect("create_session() should never share the returned session")
-                .parse_sess,
-        );
-    }
-
-    let temps_dir = sess.opts.unstable_opts.temps_dir.as_ref().map(|o| PathBuf::from(&o));
-
-    let compiler = Compiler {
-        sess,
-        codegen_backend,
-        input: config.input,
-        input_path: config.input_path,
-        output_dir: config.output_dir,
-        output_file: config.output_file,
-        temps_dir,
-        register_lints: config.register_lints,
-        override_queries: config.override_queries,
-    };
-
-    rustc_span::with_source_map(compiler.sess.parse_sess.clone_source_map(), move || {
-        let r = {
-            let _sess_abort_error = OnDrop(|| {
-                compiler.sess.finish_diagnostics(registry);
-            });
-
-            f(&compiler)
-        };
-
-        let prof = compiler.sess.prof.clone();
-        prof.generic_activity("drop_compiler").run(move || drop(compiler));
-        r
-    })
-}
-
 // JUSTIFICATION: before session exists, only config
 #[allow(rustc::bad_opt_access)]
 pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Send) -> R {
@@ -334,7 +282,53 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
     util::run_in_thread_pool_with_globals(
         config.opts.edition,
         config.opts.unstable_opts.threads,
-        || create_compiler_and_run(config, f),
+        || {
+            crate::callbacks::setup_callbacks();
+
+            let registry = &config.registry;
+            let (mut sess, codegen_backend) = util::create_session(
+                config.opts,
+                config.crate_cfg,
+                config.crate_check_cfg,
+                config.file_loader,
+                config.input_path.clone(),
+                config.lint_caps,
+                config.make_codegen_backend,
+                registry.clone(),
+            );
+
+            if let Some(parse_sess_created) = config.parse_sess_created {
+                parse_sess_created(&mut sess.parse_sess);
+            }
+
+            let temps_dir = sess.opts.unstable_opts.temps_dir.as_ref().map(|o| PathBuf::from(&o));
+
+            let compiler = Compiler {
+                sess: Lrc::new(sess),
+                codegen_backend: Lrc::new(codegen_backend),
+                input: config.input,
+                input_path: config.input_path,
+                output_dir: config.output_dir,
+                output_file: config.output_file,
+                temps_dir,
+                register_lints: config.register_lints,
+                override_queries: config.override_queries,
+            };
+
+            rustc_span::with_source_map(compiler.sess.parse_sess.clone_source_map(), move || {
+                let r = {
+                    let _sess_abort_error = OnDrop(|| {
+                        compiler.sess.finish_diagnostics(registry);
+                    });
+
+                    f(&compiler)
+                };
+
+                let prof = compiler.sess.prof.clone();
+                prof.generic_activity("drop_compiler").run(move || drop(compiler));
+                r
+            })
+        },
     )
 }
 

compiler/rustc_interface/src/util.rs

@@ -5,7 +5,6 @@ use rustc_codegen_ssa::traits::CodegenBackend;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 #[cfg(parallel_compiler)]
 use rustc_data_structures::jobserver;
-use rustc_data_structures::sync::Lrc;
 use rustc_errors::registry::Registry;
 #[cfg(parallel_compiler)]
 use rustc_middle::ty::tls;
@@ -72,7 +71,7 @@ pub fn create_session(
         Box<dyn FnOnce(&config::Options) -> Box<dyn CodegenBackend> + Send>,
     >,
     descriptions: Registry,
-) -> (Lrc<Session>, Lrc<Box<dyn CodegenBackend>>) {
+) -> (Session, Box<dyn CodegenBackend>) {
     let codegen_backend = if let Some(make_codegen_backend) = make_codegen_backend {
         make_codegen_backend(&sopts)
     } else {
@@ -119,7 +118,7 @@ pub fn create_session(
     sess.parse_sess.config = cfg;
     sess.parse_sess.check_config = check_cfg;
 
-    (Lrc::new(sess), Lrc::new(codegen_backend))
+    (sess, codegen_backend)
 }
 
 const STACK_SIZE: usize = 8 * 1024 * 1024;
@@ -130,33 +129,31 @@ fn get_stack_size() -> Option<usize> {
     env::var_os("RUST_MIN_STACK").is_none().then_some(STACK_SIZE)
 }
 
-/// Like a `thread::Builder::spawn` followed by a `join()`, but avoids the need
-/// for `'static` bounds.
 #[cfg(not(parallel_compiler))]
-fn scoped_thread<F: FnOnce() -> R + Send, R: Send>(cfg: thread::Builder, f: F) -> R {
+pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
-    // SAFETY: join() is called immediately, so any closure captures are still
-    // alive.
-    match unsafe { cfg.spawn_unchecked(f) }.unwrap().join() {
-        Ok(v) => v,
-        Err(e) => panic::resume_unwind(e),
-    }
-}
-
-#[cfg(not(parallel_compiler))]
-pub fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
     edition: Edition,
     _threads: usize,
     f: F,
 ) -> R {
-    let mut cfg = thread::Builder::new().name("rustc".to_string());
+    // 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);
+        }
 
-    if let Some(size) = get_stack_size() {
+        // `unwrap` is ok here because `spawn_scoped` only panics if the thread
-        cfg = cfg.stack_size(size);
+        // name contains null bytes.
-    }
+        let r = builder
+            .spawn_scoped(s, move || rustc_span::create_session_globals_then(edition, f))
+            .unwrap()
+            .join();
 
-    let main_handler = move || rustc_span::create_session_globals_then(edition, f);
+        match r {
-
+            Ok(v) => v,
-    scoped_thread(cfg, main_handler)
+            Err(e) => panic::resume_unwind(e),
+        }
+    })
 }
 
 /// Creates a new thread and forwards information in thread locals to it.
@@ -175,7 +172,7 @@ unsafe fn handle_deadlock() {
 }
 
 #[cfg(parallel_compiler)]
-pub fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
+pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
     edition: Edition,
     threads: usize,
     f: F,

src/librustdoc/config.rs

@@ -142,8 +142,6 @@ pub(crate) struct Options {
     // Options that alter generated documentation pages
     /// Crate version to note on the sidebar of generated docs.
     pub(crate) crate_version: Option<String>,
-    /// Collected options specific to outputting final pages.
-    pub(crate) render_options: RenderOptions,
     /// The format that we output when rendering.
     ///
     /// Currently used only for the `--show-coverage` option.
@@ -159,6 +157,10 @@ pub(crate) struct Options {
     /// Configuration for scraping examples from the current crate. If this option is Some(..) then
     /// the compiler will scrape examples and not generate documentation.
     pub(crate) scrape_examples_options: Option<ScrapeExamplesOptions>,
+
+    /// Note: this field is duplicated in `RenderOptions` because it's useful
+    /// to have it in both places.
+    pub(crate) unstable_features: rustc_feature::UnstableFeatures,
 }
 
 impl fmt::Debug for Options {
@@ -194,7 +196,6 @@ impl fmt::Debug for Options {
             .field("persist_doctests", &self.persist_doctests)
             .field("show_coverage", &self.show_coverage)
             .field("crate_version", &self.crate_version)
-            .field("render_options", &self.render_options)
             .field("runtool", &self.runtool)
             .field("runtool_args", &self.runtool_args)
             .field("enable-per-target-ignores", &self.enable_per_target_ignores)
@@ -202,6 +203,7 @@ impl fmt::Debug for Options {
             .field("no_run", &self.no_run)
             .field("nocapture", &self.nocapture)
             .field("scrape_examples_options", &self.scrape_examples_options)
+            .field("unstable_features", &self.unstable_features)
             .finish()
     }
 }
@@ -267,6 +269,8 @@ pub(crate) struct RenderOptions {
     pub(crate) generate_redirect_map: bool,
     /// Show the memory layout of types in the docs.
     pub(crate) show_type_layout: bool,
+    /// Note: this field is duplicated in `Options` because it's useful to have
+    /// it in both places.
     pub(crate) unstable_features: rustc_feature::UnstableFeatures,
     pub(crate) emit: Vec<EmitType>,
     /// If `true`, HTML source pages will generate links for items to their definition.
@@ -316,7 +320,7 @@ impl Options {
     pub(crate) fn from_matches(
         matches: &getopts::Matches,
         args: Vec<String>,
-    ) -> Result<Options, i32> {
+    ) -> Result<(Options, RenderOptions), i32> {
         let args = &args[1..];
         // Check for unstable options.
         nightly_options::check_nightly_options(matches, &opts());
@@ -710,7 +714,9 @@ impl Options {
         let with_examples = matches.opt_strs("with-examples");
         let call_locations = crate::scrape_examples::load_call_locations(with_examples, &diag)?;
 
-        Ok(Options {
+        let unstable_features =
+            rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref());
+        let options = Options {
             input,
             proc_macro_crate,
             error_format,
@@ -744,42 +750,42 @@ impl Options {
             run_check,
             no_run,
             nocapture,
-            render_options: RenderOptions {
-                output,
-                external_html,
-                id_map,
-                playground_url,
-                module_sorting,
-                themes,
-                extension_css,
-                extern_html_root_urls,
-                extern_html_root_takes_precedence,
-                default_settings,
-                resource_suffix,
-                enable_minification,
-                enable_index_page,
-                index_page,
-                static_root_path,
-                markdown_no_toc,
-                markdown_css,
-                markdown_playground_url,
-                document_private,
-                document_hidden,
-                generate_redirect_map,
-                show_type_layout,
-                unstable_features: rustc_feature::UnstableFeatures::from_environment(
-                    crate_name.as_deref(),
-                ),
-                emit,
-                generate_link_to_definition,
-                call_locations,
-                no_emit_shared: false,
-            },
             crate_name,
             output_format,
             json_unused_externs,
             scrape_examples_options,
-        })
+            unstable_features,
+        };
+        let render_options = RenderOptions {
+            output,
+            external_html,
+            id_map,
+            playground_url,
+            module_sorting,
+            themes,
+            extension_css,
+            extern_html_root_urls,
+            extern_html_root_takes_precedence,
+            default_settings,
+            resource_suffix,
+            enable_minification,
+            enable_index_page,
+            index_page,
+            static_root_path,
+            markdown_no_toc,
+            markdown_css,
+            markdown_playground_url,
+            document_private,
+            document_hidden,
+            generate_redirect_map,
+            show_type_layout,
+            unstable_features,
+            emit,
+            generate_link_to_definition,
+            call_locations,
+            no_emit_shared: false,
+        };
+        Ok((options, render_options))
     }
 
     /// Returns `true` if the file given as `self.input` is a Markdown file.

src/librustdoc/doctest.rs

@@ -18,7 +18,6 @@ use rustc_session::{lint, Session};
 use rustc_span::edition::Edition;
 use rustc_span::source_map::SourceMap;
 use rustc_span::symbol::sym;
-use rustc_span::Symbol;
 use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP};
 use rustc_target::spec::TargetTriple;
 use tempfile::Builder as TempFileBuilder;
@@ -80,7 +79,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
         lint_cap: Some(options.lint_cap.unwrap_or(lint::Forbid)),
         cg: options.codegen_options.clone(),
         externs: options.externs.clone(),
-        unstable_features: options.render_options.unstable_features,
+        unstable_features: options.unstable_features,
         actually_rustdoc: true,
         edition: options.edition,
         target_triple: options.target.clone(),
@@ -124,7 +123,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> {
                     let opts = scrape_test_config(crate_attrs);
                     let enable_per_target_ignores = options.enable_per_target_ignores;
                     let mut collector = Collector::new(
-                        tcx.crate_name(LOCAL_CRATE),
+                        tcx.crate_name(LOCAL_CRATE).to_string(),
                         options,
                         false,
                         opts,
@@ -908,7 +907,7 @@ pub(crate) struct Collector {
     rustdoc_options: RustdocOptions,
     use_headers: bool,
     enable_per_target_ignores: bool,
-    crate_name: Symbol,
+    crate_name: String,
     opts: GlobalTestOptions,
     position: Span,
     source_map: Option<Lrc<SourceMap>>,
@@ -920,7 +919,7 @@ pub(crate) struct Collector {
 
 impl Collector {
     pub(crate) fn new(
-        crate_name: Symbol,
+        crate_name: String,
         rustdoc_options: RustdocOptions,
         use_headers: bool,
         opts: GlobalTestOptions,
@@ -983,7 +982,7 @@ impl Tester for Collector {
     fn add_test(&mut self, test: String, config: LangString, line: usize) {
         let filename = self.get_filename();
         let name = self.generate_name(line, &filename);
-        let crate_name = self.crate_name.to_string();
+        let crate_name = self.crate_name.clone();
         let opts = self.opts.clone();
         let edition = config.edition.unwrap_or(self.rustdoc_options.edition);
         let rustdoc_options = self.rustdoc_options.clone();

src/librustdoc/html/render/context.rs

@@ -430,7 +430,6 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             extension_css,
             resource_suffix,
             static_root_path,
-            unstable_features,
             generate_redirect_map,
             show_type_layout,
             generate_link_to_definition,
@@ -511,7 +510,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             resource_suffix,
             static_root_path,
             fs: DocFS::new(sender),
-            codes: ErrorCodes::from(unstable_features.is_nightly_build()),
+            codes: ErrorCodes::from(options.unstable_features.is_nightly_build()),
             playground,
             all: RefCell::new(AllTypes::new()),
             errors: receiver,

src/librustdoc/lib.rs

@@ -674,39 +674,6 @@ fn usage(argv0: &str) {
 /// A result type used by several functions under `main()`.
 type MainResult = Result<(), ErrorGuaranteed>;
 
-fn main_args(at_args: &[String]) -> MainResult {
-    let args = rustc_driver::args::arg_expand_all(at_args);
-
-    let mut options = getopts::Options::new();
-    for option in opts() {
-        (option.apply)(&mut options);
-    }
-    let matches = match options.parse(&args[1..]) {
-        Ok(m) => m,
-        Err(err) => {
-            early_error(ErrorOutputType::default(), &err.to_string());
-        }
-    };
-
-    // Note that we discard any distinction between different non-zero exit
-    // codes from `from_matches` here.
-    let options = match config::Options::from_matches(&matches, args) {
-        Ok(opts) => opts,
-        Err(code) => {
-            return if code == 0 {
-                Ok(())
-            } else {
-                Err(ErrorGuaranteed::unchecked_claim_error_was_emitted())
-            };
-        }
-    };
-    rustc_interface::util::run_in_thread_pool_with_globals(
-        options.edition,
-        1, // this runs single-threaded, even in a parallel compiler
-        move || main_options(options),
-    )
-}
-
 fn wrap_return(diag: &rustc_errors::Handler, res: Result<(), String>) -> MainResult {
     match res {
         Ok(()) => Ok(()),
@@ -737,7 +704,33 @@ fn run_renderer<'tcx, T: formats::FormatRenderer<'tcx>>(
     }
 }
 
-fn main_options(options: config::Options) -> MainResult {
+fn main_args(at_args: &[String]) -> MainResult {
+    let args = rustc_driver::args::arg_expand_all(at_args);
+
+    let mut options = getopts::Options::new();
+    for option in opts() {
+        (option.apply)(&mut options);
+    }
+    let matches = match options.parse(&args[1..]) {
+        Ok(m) => m,
+        Err(err) => {
+            early_error(ErrorOutputType::default(), &err.to_string());
+        }
+    };
+
+    // Note that we discard any distinction between different non-zero exit
+    // codes from `from_matches` here.
+    let (options, render_options) = match config::Options::from_matches(&matches, args) {
+        Ok(opts) => opts,
+        Err(code) => {
+            return if code == 0 {
+                Ok(())
+            } else {
+                Err(ErrorGuaranteed::unchecked_claim_error_was_emitted())
+            };
+        }
+    };
+
     let diag = core::new_handler(
         options.error_format,
         None,
@@ -749,9 +742,18 @@ fn main_options(options: config::Options) -> MainResult {
         (true, true) => return wrap_return(&diag, markdown::test(options)),
         (true, false) => return doctest::run(options),
         (false, true) => {
+            let input = options.input.clone();
+            let edition = options.edition;
+            let config = core::create_config(options);
+
+            // `markdown::render` can invoke `doctest::make_test`, which
+            // requires session globals and a thread pool, so we use
+            // `run_compiler`.
             return wrap_return(
                 &diag,
-                markdown::render(&options.input, options.render_options, options.edition),
+                interface::run_compiler(config, |_compiler| {
+                    markdown::render(&input, render_options, edition)
+                }),
             );
         }
         (false, false) => {}
@@ -772,14 +774,12 @@ fn main_options(options: config::Options) -> MainResult {
     let crate_version = options.crate_version.clone();
 
     let output_format = options.output_format;
-    // FIXME: fix this clone (especially render_options)
     let externs = options.externs.clone();
-    let render_options = options.render_options.clone();
     let scrape_examples_options = options.scrape_examples_options.clone();
-    let document_private = options.render_options.document_private;
+
     let config = core::create_config(options);
 
-    interface::create_compiler_and_run(config, |compiler| {
+    interface::run_compiler(config, |compiler| {
         let sess = compiler.session();
 
         if sess.opts.describe_lints {
@@ -811,7 +811,7 @@ fn main_options(options: config::Options) -> MainResult {
                         sess,
                         krate,
                         externs,
-                        document_private,
+                        render_options.document_private,
                     )
                 });
                 (resolver.clone(), resolver_caches)

src/librustdoc/markdown.rs

@@ -5,7 +5,6 @@ use std::path::Path;
 
 use rustc_span::edition::Edition;
 use rustc_span::source_map::DUMMY_SP;
-use rustc_span::Symbol;
 
 use crate::config::{Options, RenderOptions};
 use crate::doctest::{Collector, GlobalTestOptions};
@@ -36,6 +35,8 @@ fn extract_leading_metadata(s: &str) -> (Vec<&str>, &str) {
 
 /// Render `input` (e.g., "foo.md") into an HTML file in `output`
 /// (e.g., output = "bar" => "bar/foo.html").
+///
+/// Requires session globals to be available, for symbol interning.
 pub(crate) fn render<P: AsRef<Path>>(
     input: P,
     options: RenderOptions,
@@ -133,7 +134,7 @@ pub(crate) fn test(options: Options) -> Result<(), String> {
     let mut opts = GlobalTestOptions::default();
     opts.no_crate_inject = true;
     let mut collector = Collector::new(
-        Symbol::intern(&options.input.display().to_string()),
+        options.input.display().to_string(),
         options.clone(),
         true,
         opts,
@@ -142,7 +143,7 @@ pub(crate) fn test(options: Options) -> Result<(), String> {
         options.enable_per_target_ignores,
     );
     collector.set_position(DUMMY_SP);
-    let codes = ErrorCodes::from(options.render_options.unstable_features.is_nightly_build());
+    let codes = ErrorCodes::from(options.unstable_features.is_nightly_build());
 
     find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None);