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

Speed up bootstrap a little. #73352

Merged
merged 4 commits into from
Jun 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 41 additions & 11 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,21 @@ struct StepDescription {
name: &'static str,
}

/// Collection of paths used to match a task rule.
#[derive(Debug, Clone, PartialOrd, Ord, PartialEq, Eq)]
pub enum PathSet {
/// A collection of individual paths.
///
/// These are generally matched as a path suffix. For example, a
/// command-line value of `libstd` will match if `src/libstd` is in the
/// set.
Set(BTreeSet<PathBuf>),
/// A "suite" of paths.
///
/// These can match as a path suffix (like `Set`), or as a prefix. For
/// example, a command-line value of `src/test/ui/abi/variadic-ffi.rs`
/// will match `src/test/ui`. A command-line value of `ui` would also
/// match `src/test/ui`.
Suite(PathBuf),
}

Expand Down Expand Up @@ -249,21 +261,33 @@ impl<'a> ShouldRun<'a> {
self
}

// Unlike `krate` this will create just one pathset. As such, it probably shouldn't actually
// ever be used, but as we transition to having all rules properly handle passing krate(...) by
// actually doing something different for every crate passed.
/// Indicates it should run if the command-line selects the given crate or
/// any of its (local) dependencies.
///
/// Compared to `krate`, this treats the dependencies as aliases for the
/// same job. Generally it is preferred to use `krate`, and treat each
/// individual path separately. For example `./x.py test src/liballoc`
/// (which uses `krate`) will test just `liballoc`. However, `./x.py check
/// src/liballoc` (which uses `all_krates`) will check all of `libtest`.
/// `all_krates` should probably be removed at some point.
pub fn all_krates(mut self, name: &str) -> Self {
let mut set = BTreeSet::new();
for krate in self.builder.in_tree_crates(name) {
set.insert(PathBuf::from(&krate.path));
let path = krate.local_path(self.builder);
set.insert(path);
}
self.paths.insert(PathSet::Set(set));
self
}

/// Indicates it should run if the command-line selects the given crate or
/// any of its (local) dependencies.
///
/// `make_run` will be called separately for each matching command-line path.
pub fn krate(mut self, name: &str) -> Self {
for krate in self.builder.in_tree_crates(name) {
self.paths.insert(PathSet::one(&krate.path));
let path = krate.local_path(self.builder);
self.paths.insert(PathSet::one(path));
}
self
}
Expand Down Expand Up @@ -487,13 +511,19 @@ impl<'a> Builder<'a> {
should_run = (desc.should_run)(should_run);
}
let mut help = String::from("Available paths:\n");
let mut add_path = |path: &Path| {
help.push_str(&format!(" ./x.py {} {}\n", subcommand, path.display()));
};
for pathset in should_run.paths {
if let PathSet::Set(set) = pathset {
set.iter().for_each(|path| {
help.push_str(
format!(" ./x.py {} {}\n", subcommand, path.display()).as_str(),
)
})
match pathset {
PathSet::Set(set) => {
for path in set {
add_path(&path);
}
}
PathSet::Suite(path) => {
add_path(&path.join("..."));
}
}
}
Some(help)
Expand Down
20 changes: 2 additions & 18 deletions src/bootstrap/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,8 @@ impl Step for Rustc {
// Find dependencies for top level crates.
let mut compiler_crates = HashSet::new();
for root_crate in &["rustc_driver", "rustc_codegen_llvm", "rustc_codegen_ssa"] {
let interned_root_crate = INTERNER.intern_str(root_crate);
find_compiler_crates(builder, &interned_root_crate, &mut compiler_crates);
compiler_crates
.extend(builder.in_tree_crates(root_crate).into_iter().map(|krate| krate.name));
}

for krate in &compiler_crates {
Expand All @@ -564,22 +564,6 @@ impl Step for Rustc {
}
}

fn find_compiler_crates(
builder: &Builder<'_>,
name: &Interned<String>,
crates: &mut HashSet<Interned<String>>,
) {
// Add current crate.
crates.insert(*name);

// Look for dependencies.
for dep in builder.crates.get(name).unwrap().deps.iter() {
if builder.crates.get(dep).unwrap().is_local(builder) {
find_compiler_crates(builder, dep, crates);
}
}
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub struct Rustdoc {
stage: u32,
Expand Down
7 changes: 2 additions & 5 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ use std::process;
use getopts::Options;

use crate::builder::Builder;
use crate::cache::{Interned, INTERNER};
use crate::config::Config;
use crate::metadata;
use crate::{Build, DocTests};

use crate::cache::{Interned, INTERNER};

/// Deserialized version of all flags for this compile.
pub struct Flags {
pub verbose: usize, // number of -v args; each extra -v after the first is passed to Cargo
Expand Down Expand Up @@ -444,8 +442,7 @@ Arguments:
// All subcommands except `clean` can have an optional "Available paths" section
if matches.opt_present("verbose") {
let config = Config::parse(&["build".to_string()]);
let mut build = Build::new(config);
metadata::build(&mut build);
let build = Build::new(config);

let maybe_rules_help = Builder::get_help(&build, subcommand.as_str());
extra_help.push_str(maybe_rules_help.unwrap_or_default().as_str());
Expand Down
25 changes: 16 additions & 9 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,7 @@ struct Crate {
}

impl Crate {
fn is_local(&self, build: &Build) -> bool {
self.path.starts_with(&build.config.src) && !self.path.to_string_lossy().ends_with("_shim")
}

fn local_path(&self, build: &Build) -> PathBuf {
assert!(self.is_local(build));
self.path.strip_prefix(&build.config.src).unwrap().into()
}
}
Expand Down Expand Up @@ -1079,17 +1074,29 @@ impl Build {
}
}

/// Returns a Vec of all the dependencies of the given root crate,
/// including transitive dependencies and the root itself. Only includes
/// "local" crates (those in the local source tree, not from a registry).
fn in_tree_crates(&self, root: &str) -> Vec<&Crate> {
let mut ret = Vec::new();
let mut list = vec![INTERNER.intern_str(root)];
let mut visited = HashSet::new();
while let Some(krate) = list.pop() {
let krate = &self.crates[&krate];
if krate.is_local(self) {
ret.push(krate);
}
ret.push(krate);
for dep in &krate.deps {
if visited.insert(dep) && dep != "build_helper" {
// Don't include optional deps if their features are not
// enabled. Ideally this would be computed from `cargo
// metadata --features …`, but that is somewhat slow. Just
// skip `build_helper` since there aren't any operations we
// want to perform on it. In the future, we may want to
// consider just filtering all build and dev dependencies in
// metadata::build.
if visited.insert(dep)
&& dep != "build_helper"
&& (dep != "profiler_builtins" || self.config.profiler)
&& (dep != "rustc_codegen_llvm" || self.config.llvm_enabled())
{
list.push(*dep);
}
}
Expand Down
62 changes: 13 additions & 49 deletions src/bootstrap/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use std::process::Command;

Expand All @@ -12,7 +10,6 @@ use crate::{Build, Crate};
#[derive(Deserialize)]
struct Output {
packages: Vec<Package>,
resolve: Resolve,
}

#[derive(Deserialize)]
Expand All @@ -21,72 +18,39 @@ struct Package {
name: String,
source: Option<String>,
manifest_path: String,
dependencies: Vec<Dependency>,
}

#[derive(Deserialize)]
struct Resolve {
nodes: Vec<ResolveNode>,
}

#[derive(Deserialize)]
struct ResolveNode {
id: String,
dependencies: Vec<String>,
struct Dependency {
name: String,
source: Option<String>,
}

pub fn build(build: &mut Build) {
let mut resolves = Vec::new();
build_krate(&build.std_features(), build, &mut resolves, "src/libstd");
build_krate("", build, &mut resolves, "src/libtest");
build_krate(&build.rustc_features(), build, &mut resolves, "src/rustc");

let mut id2name = HashMap::with_capacity(build.crates.len());
for (name, krate) in build.crates.iter() {
id2name.insert(krate.id.clone(), name.clone());
}

for node in resolves {
let name = match id2name.get(&node.id) {
Some(name) => name,
None => continue,
};

let krate = build.crates.get_mut(name).unwrap();
for dep in node.dependencies.iter() {
let dep = match id2name.get(dep) {
Some(dep) => dep,
None => continue,
};
krate.deps.insert(*dep);
}
}
}

fn build_krate(features: &str, build: &mut Build, resolves: &mut Vec<ResolveNode>, krate: &str) {
// Run `cargo metadata` to figure out what crates we're testing.
//
// Down below we're going to call `cargo test`, but to test the right set
// of packages we're going to have to know what `-p` arguments to pass it
// to know what crates to test. Here we run `cargo metadata` to learn about
// the dependency graph and what `-p` arguments there are.
let mut cargo = Command::new(&build.initial_cargo);
cargo
.arg("metadata")
.arg("--format-version")
.arg("1")
.arg("--features")
.arg(features)
.arg("--no-deps")
.arg("--manifest-path")
.arg(build.src.join(krate).join("Cargo.toml"));
.arg(build.src.join("Cargo.toml"));
let output = output(&mut cargo);
let output: Output = serde_json::from_str(&output).unwrap();
for package in output.packages {
if package.source.is_none() {
let name = INTERNER.intern_string(package.name);
let mut path = PathBuf::from(package.manifest_path);
path.pop();
build.crates.insert(name, Crate { name, id: package.id, deps: HashSet::new(), path });
let deps = package
.dependencies
.into_iter()
.filter(|dep| dep.source.is_none())
.map(|dep| INTERNER.intern_string(dep.name))
.collect();
build.crates.insert(name, Crate { name, id: package.id, deps, path });
}
}
resolves.extend(output.resolve.nodes);
}
10 changes: 2 additions & 8 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1651,14 +1651,8 @@ impl Step for Crate {
type Output = ();
const DEFAULT: bool = true;

fn should_run(mut run: ShouldRun<'_>) -> ShouldRun<'_> {
let builder = run.builder;
for krate in run.builder.in_tree_crates("test") {
if !(krate.name.starts_with("rustc_") && krate.name.ends_with("san")) {
run = run.path(krate.local_path(&builder).to_str().unwrap());
}
}
run
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.krate("test")
}

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