Skip to content

Commit 5651192

Browse files
committed
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.
1 parent b1928aa commit 5651192

File tree

4 files changed

+53
-45
lines changed

4 files changed

+53
-45
lines changed

src/librustdoc/docfs.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
1212
use std::fs;
1313
use std::io;
14-
use std::path::Path;
14+
use std::path::{Path, PathBuf};
1515
use std::string::ToString;
1616
use std::sync::mpsc::Sender;
1717

@@ -55,17 +55,17 @@ impl DocFS {
5555
fs::create_dir_all(path)
5656
}
5757

58-
crate fn write<P, C, E>(&self, path: P, contents: C) -> Result<(), E>
58+
crate fn write<E>(
59+
&self,
60+
path: PathBuf,
61+
contents: impl 'static + Send + AsRef<[u8]>,
62+
) -> Result<(), E>
5963
where
60-
P: AsRef<Path>,
61-
C: AsRef<[u8]>,
6264
E: PathError,
6365
{
6466
if !self.sync_only && cfg!(windows) {
6567
// A possible future enhancement after more detailed profiling would
6668
// be to create the file sync so errors are reported eagerly.
67-
let path = path.as_ref().to_path_buf();
68-
let contents = contents.as_ref().to_vec();
6969
let sender = self.errors.clone().expect("can't write after closing");
7070
rayon::spawn(move || {
7171
fs::write(&path, contents).unwrap_or_else(|e| {

src/librustdoc/html/render/context.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
583583
|buf: &mut Buffer| all.print(buf),
584584
&self.shared.style_files,
585585
);
586-
self.shared.fs.write(final_file, v.as_bytes())?;
586+
self.shared.fs.write(final_file, v)?;
587587

588588
// Generating settings page.
589589
page.title = "Rustdoc settings";
@@ -605,14 +605,14 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
605605
)?,
606606
&style_files,
607607
);
608-
self.shared.fs.write(&settings_file, v.as_bytes())?;
608+
self.shared.fs.write(settings_file, v)?;
609609
if let Some(ref redirections) = self.shared.redirections {
610610
if !redirections.borrow().is_empty() {
611611
let redirect_map_path =
612612
self.dst.join(&*crate_name.as_str()).join("redirect-map.json");
613613
let paths = serde_json::to_string(&*redirections.borrow()).unwrap();
614614
self.shared.ensure_dir(&self.dst.join(&*crate_name.as_str()))?;
615-
self.shared.fs.write(&redirect_map_path, paths.as_bytes())?;
615+
self.shared.fs.write(redirect_map_path, paths)?;
616616
}
617617
}
618618

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

656656
// Render sidebar-items.js used throughout this module.
@@ -662,7 +662,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
662662
let items = self.build_sidebar_items(module);
663663
let js_dst = self.dst.join("sidebar-items.js");
664664
let v = format!("initSidebarItems({});", serde_json::to_string(&items).unwrap());
665-
scx.fs.write(&js_dst, &v)?;
665+
scx.fs.write(js_dst, v)?;
666666
}
667667
Ok(())
668668
}
@@ -696,7 +696,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
696696
let file_name = &item_path(item_type, &name.as_str());
697697
self.shared.ensure_dir(&self.dst)?;
698698
let joint_dst = self.dst.join(file_name);
699-
self.shared.fs.write(&joint_dst, buf.as_bytes())?;
699+
self.shared.fs.write(joint_dst, buf)?;
700700

701701
if !self.render_redirect_pages {
702702
self.shared.all.borrow_mut().append(full_path(self, &item), &item_type);
@@ -714,7 +714,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
714714
} else {
715715
let v = layout::redirect(file_name);
716716
let redir_dst = self.dst.join(redir_name);
717-
self.shared.fs.write(&redir_dst, v.as_bytes())?;
717+
self.shared.fs.write(redir_dst, v)?;
718718
}
719719
}
720720
}

src/librustdoc/html/render/write_shared.rs

+39-31
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,10 @@ impl Context<'_> {
105105
self.dst.join(&filename)
106106
}
107107

108-
fn write_shared<C: AsRef<[u8]>>(
108+
fn write_shared(
109109
&self,
110110
resource: SharedResource<'_>,
111-
contents: C,
111+
contents: impl 'static + Send + AsRef<[u8]>,
112112
emit: &[EmitType],
113113
) -> Result<(), Error> {
114114
if resource.should_emit(emit) {
@@ -121,25 +121,23 @@ impl Context<'_> {
121121
fn write_minify(
122122
&self,
123123
resource: SharedResource<'_>,
124-
contents: &str,
124+
contents: impl 'static + Send + AsRef<str> + AsRef<[u8]>,
125125
minify: bool,
126126
emit: &[EmitType],
127127
) -> Result<(), Error> {
128-
let tmp;
129-
let contents = if minify {
130-
tmp = if resource.extension() == Some(&OsStr::new("css")) {
128+
if minify {
129+
let contents = contents.as_ref();
130+
let contents = if resource.extension() == Some(&OsStr::new("css")) {
131131
minifier::css::minify(contents).map_err(|e| {
132132
Error::new(format!("failed to minify CSS file: {}", e), resource.path(self))
133133
})?
134134
} else {
135135
minifier::js::minify(contents)
136136
};
137-
tmp.as_bytes()
137+
self.write_shared(resource, contents, emit)
138138
} else {
139-
contents.as_bytes()
140-
};
141-
142-
self.write_shared(resource, contents, emit)
139+
self.write_shared(resource, contents, emit)
140+
}
143141
}
144142
}
145143

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

158-
// The weird `: &_` is to work around a borrowck bug: https://github.com/rust-lang/rust/issues/41078#issuecomment-293646723
159-
let write_minify = |p, c: &_| {
156+
// Minified resources are usually toolchain resources. If they're not, they should use `cx.write_minify` directly.
157+
fn write_minify(
158+
basename: &'static str,
159+
contents: impl 'static + Send + AsRef<str> + AsRef<[u8]>,
160+
cx: &Context<'_>,
161+
options: &RenderOptions,
162+
) -> Result<(), Error> {
160163
cx.write_minify(
161-
SharedResource::ToolchainSpecific { basename: p },
162-
c,
164+
SharedResource::ToolchainSpecific { basename },
165+
contents,
163166
options.enable_minification,
164167
&options.emit,
165168
)
166-
};
169+
}
170+
167171
// Toolchain resources should never be dynamic.
168172
let write_toolchain = |p: &'static _, c: &'static _| {
169173
cx.write_shared(SharedResource::ToolchainSpecific { basename: p }, c, &options.emit)
@@ -210,12 +214,12 @@ pub(super) fn write_shared(
210214
"details.undocumented > summary::before, details.rustdoc-toggle > summary::before",
211215
"toggle-plus.svg",
212216
);
213-
write_minify("rustdoc.css", &rustdoc_css)?;
217+
write_minify("rustdoc.css", rustdoc_css, cx, options)?;
214218

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

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

229233
// Handle the official themes
230234
match theme {
231-
"light" => write_minify("light.css", static_files::themes::LIGHT)?,
232-
"dark" => write_minify("dark.css", static_files::themes::DARK)?,
233-
"ayu" => write_minify("ayu.css", static_files::themes::AYU)?,
235+
"light" => write_minify("light.css", static_files::themes::LIGHT, cx, options)?,
236+
"dark" => write_minify("dark.css", static_files::themes::DARK, cx, options)?,
237+
"ayu" => write_minify("ayu.css", static_files::themes::AYU, cx, options)?,
234238
_ => {
235239
// Handle added third-party themes
236240
let filename = format!("{}.{}", theme, extension);
@@ -264,26 +268,30 @@ pub(super) fn write_shared(
264268
// Maybe we can change the representation to move this out of main.js?
265269
write_minify(
266270
"main.js",
267-
&static_files::MAIN_JS.replace(
271+
static_files::MAIN_JS.replace(
268272
"/* INSERT THEMES HERE */",
269273
&format!(" = {}", serde_json::to_string(&themes).unwrap()),
270274
),
275+
cx,
276+
options,
271277
)?;
272-
write_minify("search.js", static_files::SEARCH_JS)?;
273-
write_minify("settings.js", static_files::SETTINGS_JS)?;
278+
write_minify("search.js", static_files::SEARCH_JS, cx, options)?;
279+
write_minify("settings.js", static_files::SETTINGS_JS, cx, options)?;
274280

275281
if cx.include_sources {
276-
write_minify("source-script.js", static_files::sidebar::SOURCE_SCRIPT)?;
282+
write_minify("source-script.js", static_files::sidebar::SOURCE_SCRIPT, cx, options)?;
277283
}
278284

279285
{
280286
write_minify(
281287
"storage.js",
282-
&format!(
288+
format!(
283289
"var resourcesSuffix = \"{}\";{}",
284290
cx.shared.resource_suffix,
285291
static_files::STORAGE_JS
286292
),
293+
cx,
294+
options,
287295
)?;
288296
}
289297

@@ -292,12 +300,12 @@ pub(super) fn write_shared(
292300
// This varies based on the invocation, so it can't go through the write_minify wrapper.
293301
cx.write_minify(
294302
SharedResource::InvocationSpecific { basename: "theme.css" },
295-
&buffer,
303+
buffer,
296304
options.enable_minification,
297305
&options.emit,
298306
)?;
299307
}
300-
write_minify("normalize.css", static_files::NORMALIZE_CSS)?;
308+
write_minify("normalize.css", static_files::NORMALIZE_CSS, cx, options)?;
301309
for (name, contents) in &*FILES_UNVERSIONED {
302310
cx.write_shared(SharedResource::Unversioned { name }, contents, &options.emit)?;
303311
}
@@ -512,7 +520,7 @@ pub(super) fn write_shared(
512520
content,
513521
&cx.shared.style_files,
514522
);
515-
cx.shared.fs.write(&dst, v.as_bytes())?;
523+
cx.shared.fs.write(dst, v)?;
516524
}
517525
}
518526

@@ -602,7 +610,7 @@ pub(super) fn write_shared(
602610
}",
603611
);
604612
v.push_str("})()");
605-
cx.shared.fs.write(&mydst, &v)?;
613+
cx.shared.fs.write(mydst, v)?;
606614
}
607615
Ok(())
608616
}

src/librustdoc/html/sources.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ impl SourceCollector<'_, 'tcx> {
209209
},
210210
&self.cx.shared.style_files,
211211
);
212-
self.cx.shared.fs.write(&cur, v.as_bytes())?;
212+
self.cx.shared.fs.write(cur, v)?;
213213
self.emitted_local_sources.insert(p);
214214
Ok(())
215215
}

0 commit comments

Comments
 (0)