Skip to content

Commit

Permalink
Remove unnecessary copies when using parallel IO
Browse files Browse the repository at this point in the history
Previously, rustdoc was making lots of copies of temporary owned values.
Now, it uses the owned value wherever possible.
  • Loading branch information
jyn514 committed Aug 26, 2021
1 parent b1928aa commit 5651192
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 45 deletions.
12 changes: 6 additions & 6 deletions src/librustdoc/docfs.rs
Expand Up @@ -11,7 +11,7 @@

use std::fs;
use std::io;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::string::ToString;
use std::sync::mpsc::Sender;

Expand Down Expand Up @@ -55,17 +55,17 @@ impl DocFS {
fs::create_dir_all(path)
}

crate fn write<P, C, E>(&self, path: P, contents: C) -> Result<(), E>
crate fn write<E>(
&self,
path: PathBuf,
contents: impl 'static + Send + AsRef<[u8]>,
) -> Result<(), E>
where
P: AsRef<Path>,
C: AsRef<[u8]>,
E: PathError,
{
if !self.sync_only && cfg!(windows) {
// A possible future enhancement after more detailed profiling would
// be to create the file sync so errors are reported eagerly.
let path = path.as_ref().to_path_buf();
let contents = contents.as_ref().to_vec();
let sender = self.errors.clone().expect("can't write after closing");
rayon::spawn(move || {
fs::write(&path, contents).unwrap_or_else(|e| {
Expand Down
14 changes: 7 additions & 7 deletions src/librustdoc/html/render/context.rs
Expand Up @@ -583,7 +583,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
|buf: &mut Buffer| all.print(buf),
&self.shared.style_files,
);
self.shared.fs.write(final_file, v.as_bytes())?;
self.shared.fs.write(final_file, v)?;

// Generating settings page.
page.title = "Rustdoc settings";
Expand All @@ -605,14 +605,14 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
)?,
&style_files,
);
self.shared.fs.write(&settings_file, v.as_bytes())?;
self.shared.fs.write(settings_file, v)?;
if let Some(ref redirections) = self.shared.redirections {
if !redirections.borrow().is_empty() {
let redirect_map_path =
self.dst.join(&*crate_name.as_str()).join("redirect-map.json");
let paths = serde_json::to_string(&*redirections.borrow()).unwrap();
self.shared.ensure_dir(&self.dst.join(&*crate_name.as_str()))?;
self.shared.fs.write(&redirect_map_path, paths.as_bytes())?;
self.shared.fs.write(redirect_map_path, paths)?;
}
}

Expand Down Expand Up @@ -650,7 +650,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
if !buf.is_empty() {
self.shared.ensure_dir(&self.dst)?;
let joint_dst = self.dst.join("index.html");
scx.fs.write(&joint_dst, buf.as_bytes())?;
scx.fs.write(joint_dst, buf)?;
}

// Render sidebar-items.js used throughout this module.
Expand All @@ -662,7 +662,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
let items = self.build_sidebar_items(module);
let js_dst = self.dst.join("sidebar-items.js");
let v = format!("initSidebarItems({});", serde_json::to_string(&items).unwrap());
scx.fs.write(&js_dst, &v)?;
scx.fs.write(js_dst, v)?;
}
Ok(())
}
Expand Down Expand Up @@ -696,7 +696,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
let file_name = &item_path(item_type, &name.as_str());
self.shared.ensure_dir(&self.dst)?;
let joint_dst = self.dst.join(file_name);
self.shared.fs.write(&joint_dst, buf.as_bytes())?;
self.shared.fs.write(joint_dst, buf)?;

if !self.render_redirect_pages {
self.shared.all.borrow_mut().append(full_path(self, &item), &item_type);
Expand All @@ -714,7 +714,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
} else {
let v = layout::redirect(file_name);
let redir_dst = self.dst.join(redir_name);
self.shared.fs.write(&redir_dst, v.as_bytes())?;
self.shared.fs.write(redir_dst, v)?;
}
}
}
Expand Down
70 changes: 39 additions & 31 deletions src/librustdoc/html/render/write_shared.rs
Expand Up @@ -105,10 +105,10 @@ impl Context<'_> {
self.dst.join(&filename)
}

fn write_shared<C: AsRef<[u8]>>(
fn write_shared(
&self,
resource: SharedResource<'_>,
contents: C,
contents: impl 'static + Send + AsRef<[u8]>,
emit: &[EmitType],
) -> Result<(), Error> {
if resource.should_emit(emit) {
Expand All @@ -121,25 +121,23 @@ impl Context<'_> {
fn write_minify(
&self,
resource: SharedResource<'_>,
contents: &str,
contents: impl 'static + Send + AsRef<str> + AsRef<[u8]>,
minify: bool,
emit: &[EmitType],
) -> Result<(), Error> {
let tmp;
let contents = if minify {
tmp = if resource.extension() == Some(&OsStr::new("css")) {
if minify {
let contents = contents.as_ref();
let contents = if resource.extension() == Some(&OsStr::new("css")) {
minifier::css::minify(contents).map_err(|e| {
Error::new(format!("failed to minify CSS file: {}", e), resource.path(self))
})?
} else {
minifier::js::minify(contents)
};
tmp.as_bytes()
self.write_shared(resource, contents, emit)
} else {
contents.as_bytes()
};

self.write_shared(resource, contents, emit)
self.write_shared(resource, contents, emit)
}
}
}

Expand All @@ -155,15 +153,21 @@ pub(super) fn write_shared(
let lock_file = cx.dst.join(".lock");
let _lock = try_err!(flock::Lock::new(&lock_file, true, true, true), &lock_file);

// The weird `: &_` is to work around a borrowck bug: https://github.com/rust-lang/rust/issues/41078#issuecomment-293646723
let write_minify = |p, c: &_| {
// Minified resources are usually toolchain resources. If they're not, they should use `cx.write_minify` directly.
fn write_minify(
basename: &'static str,
contents: impl 'static + Send + AsRef<str> + AsRef<[u8]>,
cx: &Context<'_>,
options: &RenderOptions,
) -> Result<(), Error> {
cx.write_minify(
SharedResource::ToolchainSpecific { basename: p },
c,
SharedResource::ToolchainSpecific { basename },
contents,
options.enable_minification,
&options.emit,
)
};
}

// Toolchain resources should never be dynamic.
let write_toolchain = |p: &'static _, c: &'static _| {
cx.write_shared(SharedResource::ToolchainSpecific { basename: p }, c, &options.emit)
Expand Down Expand Up @@ -210,12 +214,12 @@ pub(super) fn write_shared(
"details.undocumented > summary::before, details.rustdoc-toggle > summary::before",
"toggle-plus.svg",
);
write_minify("rustdoc.css", &rustdoc_css)?;
write_minify("rustdoc.css", rustdoc_css, cx, options)?;

// Add all the static files. These may already exist, but we just
// overwrite them anyway to make sure that they're fresh and up-to-date.
write_minify("settings.css", static_files::SETTINGS_CSS)?;
write_minify("noscript.css", static_files::NOSCRIPT_CSS)?;
write_minify("settings.css", static_files::SETTINGS_CSS, cx, options)?;
write_minify("noscript.css", static_files::NOSCRIPT_CSS, cx, options)?;

// To avoid "light.css" to be overwritten, we'll first run over the received themes and only
// then we'll run over the "official" styles.
Expand All @@ -228,9 +232,9 @@ pub(super) fn write_shared(

// Handle the official themes
match theme {
"light" => write_minify("light.css", static_files::themes::LIGHT)?,
"dark" => write_minify("dark.css", static_files::themes::DARK)?,
"ayu" => write_minify("ayu.css", static_files::themes::AYU)?,
"light" => write_minify("light.css", static_files::themes::LIGHT, cx, options)?,
"dark" => write_minify("dark.css", static_files::themes::DARK, cx, options)?,
"ayu" => write_minify("ayu.css", static_files::themes::AYU, cx, options)?,
_ => {
// Handle added third-party themes
let filename = format!("{}.{}", theme, extension);
Expand Down Expand Up @@ -264,26 +268,30 @@ pub(super) fn write_shared(
// Maybe we can change the representation to move this out of main.js?
write_minify(
"main.js",
&static_files::MAIN_JS.replace(
static_files::MAIN_JS.replace(
"/* INSERT THEMES HERE */",
&format!(" = {}", serde_json::to_string(&themes).unwrap()),
),
cx,
options,
)?;
write_minify("search.js", static_files::SEARCH_JS)?;
write_minify("settings.js", static_files::SETTINGS_JS)?;
write_minify("search.js", static_files::SEARCH_JS, cx, options)?;
write_minify("settings.js", static_files::SETTINGS_JS, cx, options)?;

if cx.include_sources {
write_minify("source-script.js", static_files::sidebar::SOURCE_SCRIPT)?;
write_minify("source-script.js", static_files::sidebar::SOURCE_SCRIPT, cx, options)?;
}

{
write_minify(
"storage.js",
&format!(
format!(
"var resourcesSuffix = \"{}\";{}",
cx.shared.resource_suffix,
static_files::STORAGE_JS
),
cx,
options,
)?;
}

Expand All @@ -292,12 +300,12 @@ pub(super) fn write_shared(
// This varies based on the invocation, so it can't go through the write_minify wrapper.
cx.write_minify(
SharedResource::InvocationSpecific { basename: "theme.css" },
&buffer,
buffer,
options.enable_minification,
&options.emit,
)?;
}
write_minify("normalize.css", static_files::NORMALIZE_CSS)?;
write_minify("normalize.css", static_files::NORMALIZE_CSS, cx, options)?;
for (name, contents) in &*FILES_UNVERSIONED {
cx.write_shared(SharedResource::Unversioned { name }, contents, &options.emit)?;
}
Expand Down Expand Up @@ -512,7 +520,7 @@ pub(super) fn write_shared(
content,
&cx.shared.style_files,
);
cx.shared.fs.write(&dst, v.as_bytes())?;
cx.shared.fs.write(dst, v)?;
}
}

Expand Down Expand Up @@ -602,7 +610,7 @@ pub(super) fn write_shared(
}",
);
v.push_str("})()");
cx.shared.fs.write(&mydst, &v)?;
cx.shared.fs.write(mydst, v)?;
}
Ok(())
}
2 changes: 1 addition & 1 deletion src/librustdoc/html/sources.rs
Expand Up @@ -209,7 +209,7 @@ impl SourceCollector<'_, 'tcx> {
},
&self.cx.shared.style_files,
);
self.cx.shared.fs.write(&cur, v.as_bytes())?;
self.cx.shared.fs.write(cur, v)?;
self.emitted_local_sources.insert(p);
Ok(())
}
Expand Down

0 comments on commit 5651192

Please sign in to comment.