From 50f7520526c72beab648798255e5e7a00a38f8ea Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 14 Mar 2023 20:28:00 +0100 Subject: [PATCH 1/2] rustdoc: DocFS: Replace rayon with threadpool and enable it for all targets --- Cargo.lock | 11 ++++++++++- src/librustdoc/Cargo.toml | 4 +--- src/librustdoc/docfs.rs | 27 +++++++++++++++++++++------ 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 51332919fe755..522d91aed34e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5457,13 +5457,13 @@ dependencies = [ "itertools", "minifier", "once_cell", - "rayon", "regex", "rustdoc-json-types", "serde", "serde_json", "smallvec", "tempfile", + "threadpool", "tracing", "tracing-subscriber", "tracing-tree", @@ -6208,6 +6208,15 @@ dependencies = [ "once_cell", ] +[[package]] +name = "threadpool" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d050e60b33d41c19108b32cea32164033a9013fe3b46cbd4457559bfbf77afaa" +dependencies = [ + "num_cpus", +] + [[package]] name = "tidy" version = "0.1.0" diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 6ca6edfd3c9fe..29912b95703b2 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -20,15 +20,13 @@ smallvec = "1.8.1" tempfile = "3" tracing = "0.1" tracing-tree = "0.2.0" +threadpool = "1.8.1" [dependencies.tracing-subscriber] version = "0.3.3" default-features = false features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] -[target.'cfg(windows)'.dependencies] -rayon = "1.5.1" - [dev-dependencies] expect-test = "1.4.0" diff --git a/src/librustdoc/docfs.rs b/src/librustdoc/docfs.rs index be066bdafa14a..59393697dfec3 100644 --- a/src/librustdoc/docfs.rs +++ b/src/librustdoc/docfs.rs @@ -9,11 +9,14 @@ //! needs to read-after-write from a file, then it would be added to this //! abstraction. +use std::cmp::max; use std::fs; use std::io; use std::path::{Path, PathBuf}; use std::string::ToString; use std::sync::mpsc::Sender; +use std::thread::available_parallelism; +use threadpool::ThreadPool; pub(crate) trait PathError { fn new>(e: S, path: P) -> Self @@ -24,11 +27,21 @@ pub(crate) trait PathError { pub(crate) struct DocFS { sync_only: bool, errors: Option>, + pool: ThreadPool, } impl DocFS { pub(crate) fn new(errors: Sender) -> DocFS { - DocFS { sync_only: false, errors: Some(errors) } + const MINIMUM_NB_THREADS: usize = 2; + DocFS { + sync_only: false, + errors: Some(errors), + pool: ThreadPool::new( + available_parallelism() + .map(|nb| max(nb.get(), MINIMUM_NB_THREADS)) + .unwrap_or(MINIMUM_NB_THREADS), + ), + } } pub(crate) fn set_sync_only(&mut self, sync_only: bool) { @@ -54,12 +67,11 @@ impl DocFS { where E: PathError, { - #[cfg(windows)] if !self.sync_only { // A possible future enhancement after more detailed profiling would // be to create the file sync so errors are reported eagerly. let sender = self.errors.clone().expect("can't write after closing"); - rayon::spawn(move || { + self.pool.execute(move || { fs::write(&path, contents).unwrap_or_else(|e| { sender.send(format!("\"{}\": {}", path.display(), e)).unwrap_or_else(|_| { panic!("failed to send error on \"{}\"", path.display()) @@ -70,9 +82,12 @@ impl DocFS { fs::write(&path, contents).map_err(|e| E::new(e, path))?; } - #[cfg(not(windows))] - fs::write(&path, contents).map_err(|e| E::new(e, path))?; - Ok(()) } } + +impl Drop for DocFS { + fn drop(&mut self) { + self.pool.join(); + } +} From e667872bfdcc48305e1fae90d278693cb089223e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 15 Mar 2023 16:47:14 +0100 Subject: [PATCH 2/2] Update docsfs module documentation --- src/librustdoc/docfs.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustdoc/docfs.rs b/src/librustdoc/docfs.rs index 59393697dfec3..d58b8dc6ad4a4 100644 --- a/src/librustdoc/docfs.rs +++ b/src/librustdoc/docfs.rs @@ -2,7 +2,6 @@ //! //! On Windows this indirects IO into threads to work around performance issues //! with Defender (and other similar virus scanners that do blocking operations). -//! On other platforms this is a thin shim to fs. //! //! Only calls needed to permit this workaround have been abstracted: thus //! fs::read is still done directly via the fs module; if in future rustdoc