Skip to content

Add support for building rustc-perf and running the collector on Windows #865

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

Merged
merged 11 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
68 changes: 55 additions & 13 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions collector/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ num_cpus = "1.13"
jobserver = "0.1.21"
crossbeam-utils = "0.7"
snap = "1"
filetime = "0.2.14"
walkdir = "2"

[target.'cfg(windows)'.dependencies]
miow = "0.3"
winapi = { version = "0.3", features = ["winerror"] }

[[bin]]
name = "collector"
Expand Down
93 changes: 71 additions & 22 deletions collector/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use collector::command_output;
use database::{PatchName, QueryLabel};
use futures::stream::FuturesUnordered;
use futures::stream::StreamExt;
use std::cmp;
use std::{cmp, ffi::OsStr, path::Component};
use std::collections::HashMap;
use std::env;
use std::fmt;
Expand All @@ -23,6 +23,20 @@ use tokio::runtime::Runtime;

mod rustc;

#[cfg(windows)]
fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> anyhow::Result<()> {
let (from, to) = (from.as_ref(), to.as_ref());

let ctx = format!("renaming file {:?} to {:?}", from, to);

if fs::metadata(from)?.is_file() {
return Ok(fs::rename(from, to).with_context(|| ctx.clone())?);
}

collector::robocopy(from, to, &[&"/move"]).with_context(|| ctx.clone())
}

#[cfg(unix)]
fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> anyhow::Result<()> {
let (from, to) = (from.as_ref(), to.as_ref());
if fs::rename(from, to).is_err() {
Expand All @@ -44,26 +58,48 @@ fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> anyhow::Result<()>
Ok(())
}

fn touch(root: &Path, path: &Path) -> anyhow::Result<()> {
let mut cmd = Command::new("touch");
cmd.current_dir(root).arg(path);
command_output(&mut cmd).with_context(|| format!("touching {:?} in {:?}", path, root))?;
fn touch(path: &Path) -> anyhow::Result<()> {
log::info!("touching file {:?}", path);

filetime::set_file_mtime(path, filetime::FileTime::now()).with_context(|| format!("touching file {:?}", path))?;

Ok(())
}

fn touch_all(path: &Path) -> anyhow::Result<()> {
let mut cmd = Command::new("bash");
// Don't touch files in `target/`, since they're likely generated by build scripts and might be from a dependency.
// Don't touch build scripts, which confuses the wrapped rustc.
cmd.current_dir(path)
.args(&["-c", "find . -path ./target -prune -false -o -name '*.rs' | grep -v '^./build.rs$' | xargs touch"]);
command_output(&mut cmd).with_context(|| format!("touching all .rs in {:?}", path))?;
// We also delete the cmake caches to avoid errors when moving directories around.
// This might be a bit slower but at least things build
let mut cmd = Command::new("bash");
cmd.current_dir(path)
.args(&["-c", "find . -name 'CMakeCache.txt' -delete"]);
command_output(&mut cmd).with_context(|| format!("deleting cmake caches in {:?}", path))?;
fn is_valid(path: &Path) -> bool {
let target_dir = Component::Normal(OsStr::new("target"));

// Don't touch files in `target/`, since they're likely generated by build scripts and might be from a dependency.
if path.components().any(|component| component == target_dir) {
Copy link
Member

Choose a reason for hiding this comment

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

This would break if any subfolder is named target no?

Copy link
Member

Choose a reason for hiding this comment

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

I think that's only true if the subfolder actually needs touching; I'm not sure that will happen in practice. Realistically we can likely get away with touching the crate "roots" only, but those are a bit annoying to identify.

return false;
}

if let Some(extn) = path.extension() {
if extn.to_str() == Some("rs") {
// Don't touch build scripts, which confuses the wrapped rustc.
return path.file_name() != Some(OsStr::new("build.rs"));
}
}

false
}

for entry in walkdir::WalkDir::new(path) {
let entry = entry?;
let path = entry.path();

// We also delete the cmake caches to avoid errors when moving directories around.
// This might be a bit slower but at least things build
if path.file_name() == Some(OsStr::new("CMakeCache.txt")) {
fs::remove_file(path).with_context(|| format!("deleting cmake caches in {:?}", path))?;
}

if is_valid(path) {
touch(path)?;
}
}

Ok(())
}

Expand Down Expand Up @@ -380,7 +416,7 @@ impl<'a> CargoProcess<'a> {
// in-tree (e.g., in the case of the servo crates there are a lot of
// other components).
if let Some(file) = &self.touch_file {
touch(&self.cwd, Path::new(&file))?;
touch(&self.cwd.join(Path::new(&file)))?;
} else {
touch_all(
&self.cwd.join(
Expand Down Expand Up @@ -880,11 +916,13 @@ impl<'a> Processor for ProfileProcessor<'a> {
rename(path, filepath(&zsp_dir, "Zsp.string_data"))?;
} else if filename_str.ends_with(".string_index") {
rename(path, filepath(&zsp_dir, "Zsp.string_index"))?;
} else if filename_str.ends_with(".mm_profdata") {
rename(path, filepath(&zsp_dir, "Zsp.mm_profdata"))?;
} else {
panic!("unexpected file {:?}", path);
}
}
assert_eq!(num_files, 3);
assert!(num_files == 3 || num_files == 1);

// Run `summarize`.
let mut summarize_cmd = Command::new("summarize");
Expand Down Expand Up @@ -1095,15 +1133,26 @@ impl Benchmark {
self.config.supports_stable
}

#[cfg(windows)]
fn copy(from: &Path, to: &Path) -> anyhow::Result<()> {
collector::robocopy(from, to, &[])
}

#[cfg(unix)]
fn copy(from: &Path, to: &Path) -> anyhow::Result<()> {
let mut cmd = Command::new("cp");
cmd.arg("-pLR").arg(from).arg(to);
command_output(&mut cmd)?;
Ok(())
}

fn make_temp_dir(&self, base: &Path) -> anyhow::Result<TempDir> {
// Appending `.` means we copy just the contents of `base` into
// `tmp_dir`, rather than `base` itself.
let mut base_dot = base.to_path_buf();
base_dot.push(".");
let tmp_dir = TempDir::new()?;
let mut cmd = Command::new("cp");
cmd.arg("-pLR").arg(base_dot).arg(tmp_dir.path());
command_output(&mut cmd).with_context(|| format!("copying {} to tmp dir", self.name))?;
Self::copy(&base_dot, tmp_dir.path()).with_context(|| format!("copying {} to tmp dir", self.name))?;
Ok(tmp_dir)
}

Expand Down
Loading