Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some low-hanging rustdoc optimizations #44613

Merged
merged 2 commits into from Oct 15, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/librustdoc/html/render.rs
Expand Up @@ -125,6 +125,21 @@ pub struct SharedContext {
/// Warnings for the user if rendering would differ using different markdown
/// parsers.
pub markdown_warnings: RefCell<Vec<(Span, String, Vec<html_diff::Difference>)>>,
/// The directories that have already been created in this doc run. Used to reduce the number
/// of spurious `create_dir_all` calls.
pub created_dirs: RefCell<FxHashSet<PathBuf>>,
}

impl SharedContext {
fn ensure_dir(&self, dst: &Path) -> io::Result<()> {
let mut dirs = self.created_dirs.borrow_mut();
if !dirs.contains(dst) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This nicely avoids allocating the PathBuf if the directory is already present 👍

fs::create_dir_all(dst)?;
dirs.insert(dst.to_path_buf());
}

Ok(())
}
}

/// Indicates where an external crate can be found.
Expand Down Expand Up @@ -460,6 +475,7 @@ pub fn run(mut krate: clean::Crate,
},
css_file_extension: css_file_extension.clone(),
markdown_warnings: RefCell::new(vec![]),
created_dirs: RefCell::new(FxHashSet()),
};

// If user passed in `--playground-url` arg, we fill in crate name here
Expand Down Expand Up @@ -790,7 +806,6 @@ fn write_shared(cx: &Context,
// Write out the shared files. Note that these are shared among all rustdoc
// docs placed in the output directory, so this needs to be a synchronized
// operation with respect to all other rustdocs running around.
try_err!(fs::create_dir_all(&cx.dst), &cx.dst);
let _lock = flock::Lock::panicking_new(&cx.dst.join(".lock"), true, true, true);

// Add all the static files. These may already exist, but we just
Expand Down Expand Up @@ -1503,8 +1518,8 @@ impl Context {
this.render_item(&mut buf, &item, false).unwrap();
// buf will be empty if the module is stripped and there is no redirect for it
if !buf.is_empty() {
try_err!(this.shared.ensure_dir(&this.dst), &this.dst);
let joint_dst = this.dst.join("index.html");
try_err!(fs::create_dir_all(&this.dst), &this.dst);
let mut dst = try_err!(File::create(&joint_dst), &joint_dst);
try_err!(dst.write_all(&buf), &joint_dst);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks to me like the new version of this code does not change the amount of syscalls, since both versions write into a memory buffer first.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section (and the related one above it) managed its own buffer since render_item would bypass calling layout::render or layout::redirect if the item was stripped and didn't need a page. Since i made a global buffer i needed to add some extra handling around this section to make sure it didn't copy the buffer to another buffer before actually writing it. Mainly to save the allocation and memcpy rather than the filesystem interactions.

}
Expand Down Expand Up @@ -1538,8 +1553,8 @@ impl Context {
let name = item.name.as_ref().unwrap();
let item_type = item.type_();
let file_name = &item_path(item_type, name);
try_err!(self.shared.ensure_dir(&self.dst), &self.dst);
let joint_dst = self.dst.join(file_name);
try_err!(fs::create_dir_all(&self.dst), &self.dst);
let mut dst = try_err!(File::create(&joint_dst), &joint_dst);
try_err!(dst.write_all(&buf), &joint_dst);

Expand Down