Skip to content

Commit

Permalink
Rollup merge of #109139 - GuillaumeGomez:rustdoc-windows-wait-for-wri…
Browse files Browse the repository at this point in the history
…te, r=notriddle

rustdoc: DocFS: Replace rayon with threadpool and enable it for all targets

Fixes #109060.

Switching to `threadpool` makes it a bit simpler for us to wait for all tasks in `DocFS` directly in the `Drop` implementation. I'm also curious if making all the writes into a thread pool could improve run time for rustdoc on all other platforms than Windows as well.

I'll run a perf check to see.

cc ```@ehuss```
r? ```@notriddle```
  • Loading branch information
matthiaskrgr authored Mar 16, 2023
2 parents 1385a32 + e667872 commit 6cf2f47
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 11 deletions.
11 changes: 10 additions & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5458,13 +5458,13 @@ dependencies = [
"itertools",
"minifier",
"once_cell",
"rayon",
"regex",
"rustdoc-json-types",
"serde",
"serde_json",
"smallvec",
"tempfile",
"threadpool",
"tracing",
"tracing-subscriber",
"tracing-tree",
Expand Down Expand Up @@ -6209,6 +6209,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"
Expand Down
4 changes: 1 addition & 3 deletions src/librustdoc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
28 changes: 21 additions & 7 deletions src/librustdoc/docfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
//!
//! 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
//! 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<S, P: AsRef<Path>>(e: S, path: P) -> Self
Expand All @@ -24,11 +26,21 @@ pub(crate) trait PathError {
pub(crate) struct DocFS {
sync_only: bool,
errors: Option<Sender<String>>,
pool: ThreadPool,
}

impl DocFS {
pub(crate) fn new(errors: Sender<String>) -> 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) {
Expand All @@ -54,12 +66,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())
Expand All @@ -70,9 +81,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();
}
}

0 comments on commit 6cf2f47

Please sign in to comment.