Skip to content

Commit

Permalink
Rollup merge of rust-lang#86152 - the8472:lazify-npm-queries, r=Mark-…
Browse files Browse the repository at this point in the history
…Simulacrum

Lazify is_really_default condition in the RustdocGUI bootstrap step

The `RustdocGUI::should_run` condition spawns `npm list` several times which adds up to seconds of wall-time.
Evaluate the condition lazily to to keep `./x.py test tidy` and similar short-running tasks fast.

Fixes rust-lang#86147
  • Loading branch information
JohnTitor authored Jun 21, 2021
2 parents b491517 + bde9570 commit b9b6a09
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 8 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ dependencies = [
"libc",
"merge",
"num_cpus",
"once_cell",
"opener",
"pretty_assertions",
"serde",
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ time = "0.1"
ignore = "0.4.10"
opener = "0.4"
merge = "0.1.0"
once_cell = "1.7.2"

[target.'cfg(windows)'.dependencies.winapi]
version = "0.3"
Expand Down
28 changes: 23 additions & 5 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use crate::util::{self, add_dylib_path, add_link_lib_path, exe, libdir};
use crate::{Build, DocTests, GitRepo, Mode};

pub use crate::Compiler;
// FIXME: replace with std::lazy after it gets stabilized and reaches beta
use once_cell::sync::Lazy;

pub struct Builder<'a> {
pub build: &'a Build,
Expand Down Expand Up @@ -195,7 +197,7 @@ impl StepDescription {

if paths.is_empty() || builder.config.include_default_paths {
for (desc, should_run) in v.iter().zip(&should_runs) {
if desc.default && should_run.is_really_default {
if desc.default && should_run.is_really_default() {
for pathset in &should_run.paths {
desc.maybe_run(builder, pathset);
}
Expand Down Expand Up @@ -228,31 +230,47 @@ impl StepDescription {
}
}

#[derive(Clone)]
enum ReallyDefault<'a> {
Bool(bool),
Lazy(Lazy<bool, Box<dyn Fn() -> bool + 'a>>),
}

pub struct ShouldRun<'a> {
pub builder: &'a Builder<'a>,
// use a BTreeSet to maintain sort order
paths: BTreeSet<PathSet>,

// If this is a default rule, this is an additional constraint placed on
// its run. Generally something like compiler docs being enabled.
is_really_default: bool,
is_really_default: ReallyDefault<'a>,
}

impl<'a> ShouldRun<'a> {
fn new(builder: &'a Builder<'_>) -> ShouldRun<'a> {
ShouldRun {
builder,
paths: BTreeSet::new(),
is_really_default: true, // by default no additional conditions
is_really_default: ReallyDefault::Bool(true), // by default no additional conditions
}
}

pub fn default_condition(mut self, cond: bool) -> Self {
self.is_really_default = cond;
self.is_really_default = ReallyDefault::Bool(cond);
self
}

pub fn lazy_default_condition(mut self, lazy_cond: Box<dyn Fn() -> bool + 'a>) -> Self {
self.is_really_default = ReallyDefault::Lazy(Lazy::new(lazy_cond));
self
}

pub fn is_really_default(&self) -> bool {
match &self.is_really_default {
ReallyDefault::Bool(val) => *val,
ReallyDefault::Lazy(lazy) => *lazy.deref(),
}
}

/// Indicates it should run if the command-line selects the given crate or
/// any of its (local) dependencies.
///
Expand Down
6 changes: 3 additions & 3 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,15 +806,15 @@ impl Step for RustdocGUI {
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
let builder = run.builder;
let run = run.suite_path("src/test/rustdoc-gui");
run.default_condition(
run.lazy_default_condition(Box::new(move || {
builder.config.nodejs.is_some()
&& builder
.config
.npm
.as_ref()
.map(|p| check_if_browser_ui_test_is_installed(p))
.unwrap_or(false),
)
.unwrap_or(false)
}))
}

fn make_run(run: RunConfig<'_>) {
Expand Down

0 comments on commit b9b6a09

Please sign in to comment.