From 56511924606ab8791a50822952e3101e8030fe8e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 22 Aug 2021 02:25:25 +0000 Subject: [PATCH] Remove unnecessary copies when using parallel IO Previously, rustdoc was making lots of copies of temporary owned values. Now, it uses the owned value wherever possible. --- src/librustdoc/docfs.rs | 12 ++-- src/librustdoc/html/render/context.rs | 14 ++--- src/librustdoc/html/render/write_shared.rs | 70 ++++++++++++---------- src/librustdoc/html/sources.rs | 2 +- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/librustdoc/docfs.rs b/src/librustdoc/docfs.rs index 9b740acfcdf..a5fab1b3d42 100644 --- a/src/librustdoc/docfs.rs +++ b/src/librustdoc/docfs.rs @@ -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; @@ -55,17 +55,17 @@ impl DocFS { fs::create_dir_all(path) } - crate fn write(&self, path: P, contents: C) -> Result<(), E> + crate fn write( + &self, + path: PathBuf, + contents: impl 'static + Send + AsRef<[u8]>, + ) -> Result<(), E> where - P: AsRef, - 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| { diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 6ce0828e159..8ed15054266 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -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"; @@ -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)?; } } @@ -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. @@ -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(()) } @@ -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); @@ -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)?; } } } diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index c16769c474a..a1515b44e7c 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -105,10 +105,10 @@ impl Context<'_> { self.dst.join(&filename) } - fn write_shared>( + fn write_shared( &self, resource: SharedResource<'_>, - contents: C, + contents: impl 'static + Send + AsRef<[u8]>, emit: &[EmitType], ) -> Result<(), Error> { if resource.should_emit(emit) { @@ -121,25 +121,23 @@ impl Context<'_> { fn write_minify( &self, resource: SharedResource<'_>, - contents: &str, + contents: impl 'static + Send + AsRef + 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) + } } } @@ -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 + 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) @@ -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. @@ -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); @@ -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, )?; } @@ -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)?; } @@ -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)?; } } @@ -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(()) } diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 73916e204d9..b2180460377 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -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(()) }