diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4e738..99c1e80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Unreleased +## Create an index right after a test finishes + +With the `compile-index-and-clean` feature of the testclient, +it can invoke `cargo-difftests` to create an index immediately +after writing the profile, and then cleaning the intermediate +profiling data. This only happens if the testclient is configured +to do so (see `DifftestsEnv::and_compile_index_and_clean_on_exit`). + +This, together with the tiny indexes feature released in `0.5.0`, +have the effect of a huge reduction in the on-disk space required +to store profiling data, only a couple KBs/test (depending on project +of course; on the sample project, the average is somewhere around 700 +bytes / test). + # 0.5.0 Released: 2023-02-03 diff --git a/cargo-difftests-testclient/Cargo.toml b/cargo-difftests-testclient/Cargo.toml index a43c0ef..913bfb6 100644 --- a/cargo-difftests-testclient/Cargo.toml +++ b/cargo-difftests-testclient/Cargo.toml @@ -15,6 +15,7 @@ default = ["enforce-single-running-test", "groups", "parallel-groups"] enforce-single-running-test = [] groups = ["enforce-single-running-test", "cargo-difftests-core/groups"] parallel-groups = ["groups"] +compile-index-and-clean = [] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/cargo-difftests-testclient/src/compile_index_and_clean_config.rs b/cargo-difftests-testclient/src/compile_index_and_clean_config.rs new file mode 100644 index 0000000..ccfc9b4 --- /dev/null +++ b/cargo-difftests-testclient/src/compile_index_and_clean_config.rs @@ -0,0 +1,208 @@ +use std::path::PathBuf; + +pub struct CompileIndexAndCleanConfig { + difftests_dir: PathBuf, + index_resolver: IndexResolver, + other_bins: Vec, + flatten_files_to: Option, + remove_bin_path: bool, + full_index: bool, + #[cfg(windows)] + path_slash_replace: bool, +} + +pub enum FlattenFilesToTarget { + RepoRoot, +} + +enum IndexResolver { + FromRoots { + index_root: PathBuf, + difftests_root: PathBuf, + }, + Path { + index_path: PathBuf, + }, +} + +pub struct CompileIndexAndCleanConfigBuilder { + difftests_dir: PathBuf, + index_resolver: Option, + other_bins: Vec, + flatten_files_to: Option, + remove_bin_path: bool, + full_index: bool, + #[cfg(windows)] + path_slash_replace: bool, +} + +impl CompileIndexAndCleanConfigBuilder { + pub fn new(difftests_dir: PathBuf) -> Self { + Self { + difftests_dir, + index_resolver: None, + other_bins: Vec::new(), + flatten_files_to: None, + remove_bin_path: true, + full_index: false, + #[cfg(windows)] + path_slash_replace: true, + } + } + + pub fn index_path(mut self, index_path: impl Into) -> Self { + match &mut self.index_resolver { + Some(_) => panic!("index_path or roots already set"), + None => { + self.index_resolver = Some(IndexResolver::Path { + index_path: index_path.into(), + }); + } + } + self + } + + pub fn roots(mut self, index_root: impl Into, difftests_root: impl Into) -> Self { + match &mut self.index_resolver { + Some(_) => panic!("index_path or roots already set"), + None => { + self.index_resolver = Some(IndexResolver::FromRoots { + index_root: index_root.into(), + difftests_root: difftests_root.into(), + }); + } + } + self + } + + pub fn other_bins(mut self, other_bins: impl IntoIterator) -> Self { + self.other_bins.extend(other_bins); + self + } + + pub fn flatten_files_to( + mut self, + flatten_files_to: impl Into>, + ) -> Self { + self.flatten_files_to = flatten_files_to.into(); + self + } + + pub fn remove_bin_path(mut self, remove_bin_path: bool) -> Self { + self.remove_bin_path = remove_bin_path; + self + } + + pub fn full_index(mut self, full_index: bool) -> Self { + self.full_index = full_index; + self + } + + #[cfg(windows)] + pub fn path_slash_replace(mut self, path_slash_replace: bool) -> Self { + self.path_slash_replace = path_slash_replace; + self + } + + #[track_caller] + pub fn build(self) -> CompileIndexAndCleanConfig { + CompileIndexAndCleanConfig { + difftests_dir: self.difftests_dir, + index_resolver: self.index_resolver.unwrap(), + other_bins: self.other_bins, + flatten_files_to: self.flatten_files_to, + remove_bin_path: self.remove_bin_path, + full_index: self.full_index, + #[cfg(windows)] + path_slash_replace: self.path_slash_replace, + } + } +} + +pub fn do_build_index_and_clean(config: &CompileIndexAndCleanConfig) -> std::io::Result<()> { + let mut cmd = std::process::Command::new(env!("CARGO")); + + cmd.args(&[ + "difftests", + "low-level", + "test-client-compile-test-index-and-clean", + ]); + + cmd.arg("--dir").arg(&config.difftests_dir); + + match &config.index_resolver { + IndexResolver::FromRoots { + index_root, + difftests_root, + } => { + cmd.arg("--index-root").arg(index_root); + cmd.arg("--root").arg(difftests_root); + cmd.arg("--output").arg("resolve"); + } + IndexResolver::Path { index_path } => { + cmd.arg("--output").arg(index_path); + } + } + + if let Some(flatten_files_to) = &config.flatten_files_to { + match flatten_files_to { + FlattenFilesToTarget::RepoRoot => { + cmd.arg("--flatten-files-to=repo-root"); + } + } + } + + if !config.remove_bin_path { + cmd.arg("--no-remove-bin-path"); + } + + if config.full_index { + cmd.arg("--full-index"); + } + + #[cfg(windows)] + { + if !config.path_slash_replace { + cmd.arg("--no-path-slash-replace"); + } + } + + for bin in &config.other_bins { + cmd.arg("--bin").arg(bin); + } + + cmd.stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()); + + let output = cmd.output()?; + + if !output.status.success() { + use std::io::Write; + + println!("cargo-difftests test-client-compile-test-index-and-clean stdout:"); + std::io::stdout().write_all(&output.stdout)?; + eprintln!("cargo-difftests test-client-compile-test-index-and-clean stderr:"); + std::io::stderr().write_all(&output.stderr)?; + + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + "cargo difftests low-level compile-test-index failed", + )); + } + + Ok(()) +} + +pub struct RunOnDrop(CompileIndexAndCleanConfig); + +impl RunOnDrop { + pub fn new(config: CompileIndexAndCleanConfig) -> Self { + Self(config) + } +} + +impl Drop for RunOnDrop { + fn drop(&mut self) { + do_build_index_and_clean(&self.0).unwrap(); + } +} diff --git a/cargo-difftests-testclient/src/lib.rs b/cargo-difftests-testclient/src/lib.rs index 7826070..4b8fd4e 100644 --- a/cargo-difftests-testclient/src/lib.rs +++ b/cargo-difftests-testclient/src/lib.rs @@ -60,6 +60,9 @@ impl Drop for SelfProfileWriter { } } +#[cfg(feature = "compile-index-and-clean")] +pub mod compile_index_and_clean_config; + enum DifftestsEnvInner { Test { #[allow(dead_code)] @@ -70,9 +73,15 @@ enum DifftestsEnvInner { not(feature = "parallel-groups") ))] _t_lock: std::sync::MutexGuard<'static, ()>, + + #[cfg(feature = "compile-index-and-clean")] + compile_index_and_clean: Option, + + #[cfg(feature = "compile-index-and-clean")] + self_dir: PathBuf, }, #[cfg(feature = "groups")] - Group(groups::GroupDifftestsEnv), + Group(#[allow(dead_code)] groups::GroupDifftestsEnv), } #[cfg(feature = "parallel-groups")] @@ -118,6 +127,35 @@ impl DifftestsEnv { self.llvm_profile_file_value.as_os_str(), )) } + + #[cfg(feature = "compile-index-and-clean")] + pub fn and_compile_index_and_clean_on_exit( + mut self, + f: impl FnOnce( + compile_index_and_clean_config::CompileIndexAndCleanConfigBuilder, + ) -> compile_index_and_clean_config::CompileIndexAndCleanConfigBuilder, + ) -> Self { + match &mut self.difftests_env_inner { + DifftestsEnvInner::Test { + compile_index_and_clean, + self_dir, + .. + } => { + let c = compile_index_and_clean_config::CompileIndexAndCleanConfigBuilder::new( + self_dir.clone(), + ); + let c = f(c); + *compile_index_and_clean = Some(compile_index_and_clean_config::RunOnDrop::new(c.build())); + } + DifftestsEnvInner::Group { .. } => { + panic!( + "and_compile_index_on_exit can only be called in a non-group test environment" + ); + } + } + + self + } } /// Initializes the difftests environment. @@ -177,6 +215,12 @@ pub fn init( not(feature = "parallel-groups") ))] _t_lock, + + #[cfg(feature = "compile-index-and-clean")] + compile_index_and_clean: None, + + #[cfg(feature = "compile-index-and-clean")] + self_dir: tmpdir.to_path_buf(), }, }); diff --git a/cargo-difftests/src/main.rs b/cargo-difftests/src/main.rs index c6be8bd..c8833b4 100644 --- a/cargo-difftests/src/main.rs +++ b/cargo-difftests/src/main.rs @@ -17,6 +17,7 @@ #![feature(exit_status_error)] use core::fmt; +use std::ffi::OsString; use std::fmt::{Display, Formatter}; use std::fs; use std::io::{BufRead, Write}; @@ -211,6 +212,28 @@ pub enum LowLevelCommand { #[clap(flatten)] ignore_registry_files: IgnoreRegistryFilesFlag, }, + /// Compile a test index for a single difftest directory, then + /// clean the difftest directory. (internal: only used from the testclient). + TestClientCompileTestIndexAndClean { + #[clap(flatten)] + dir: DifftestDir, + /// The output file to write the index to. + #[clap(short, long)] + output: IndexPathOrResolve, + #[clap(flatten)] + export_profdata_config_flags: ExportProfdataConfigFlags, + #[clap(flatten)] + compile_test_index_flags: CompileTestIndexFlags, + #[clap(flatten)] + ignore_registry_files: IgnoreRegistryFilesFlag, + + /// The root directory where all the difftests were stored. + #[clap(long, required_if_eq("output", "resolve"))] + root: Option, + /// The directory where the index files were stored, if any. + #[clap(long, required_if_eq("output", "resolve"))] + index_root: Option, + }, /// Runs the analysis for a single test index. RunAnalysisWithTestIndex { /// The path to the test index. @@ -232,6 +255,22 @@ pub enum LowLevelCommand { }, } +#[derive(Debug, Clone)] +pub enum IndexPathOrResolve { + Resolve, + Path(PathBuf), +} + +impl From for IndexPathOrResolve { + fn from(s: OsString) -> Self { + if s == "resolve" { + IndexPathOrResolve::Resolve + } else { + IndexPathOrResolve::Path(PathBuf::from(s)) + } + } +} + #[derive(ValueEnum, Debug, Copy, Clone, Default)] pub enum IndexesTouchSameFilesReportAction { /// Print the report to stdout. @@ -964,6 +1003,52 @@ fn run_compile_test_index( Ok(()) } +fn run_test_client_compile_test_index_and_clean( + dir: PathBuf, + output: IndexPathOrResolve, + export_profdata_config_flags: ExportProfdataConfigFlags, + compile_test_index_flags: CompileTestIndexFlags, + ignore_registry_files: IgnoreRegistryFilesFlag, + root: Option, + index_root: Option, +) -> CargoDifftestsResult { + let index_resolver = match output { + IndexPathOrResolve::Resolve => { + resolver_for_index_root(root.as_ref().map(PathBuf::as_path).unwrap(), index_root) + .unwrap() + } + IndexPathOrResolve::Path(p) => DiscoverIndexPathResolver::Custom { + f: Box::new(move |_| Some(p.clone())), + }, + }; + + let mut discovered = Difftest::discover_from(dir.clone(), Some(&index_resolver))?; + discovered.merge_profraw_files_into_profdata(false)?; + + assert!(discovered.has_profdata()); + + let config = compile_test_index_config(compile_test_index_flags, ignore_registry_files)?; + + let result = discovered.compile_test_index_data( + export_profdata_config_flags.config(ignore_registry_files), + config, + )?; + + let output = index_resolver.resolve(&dir).unwrap(); + + if let Some(parent) = output.parent() { + if !parent.exists() { + fs::create_dir_all(parent)?; + } + } + + result.write_to_file(&output)?; + + discovered.clean()?; + + Ok(()) +} + fn run_indexes_touch_same_files_report( index1: PathBuf, index2: PathBuf, @@ -1008,6 +1093,25 @@ fn run_low_level_cmd(ctxt: &CargoDifftestsContext, cmd: LowLevelCommand) -> Carg ignore_registry_files, )?; } + LowLevelCommand::TestClientCompileTestIndexAndClean { + dir, + output, + export_profdata_config_flags, + compile_test_index_flags, + ignore_registry_files, + root, + index_root, + } => { + run_test_client_compile_test_index_and_clean( + dir.dir, + output, + export_profdata_config_flags, + compile_test_index_flags, + ignore_registry_files, + root, + index_root, + )?; + } LowLevelCommand::RunAnalysisWithTestIndex { index, algo: AlgoArgs { algo, commit }, diff --git a/sample/cargo-difftests-sample-project/Cargo.toml b/sample/cargo-difftests-sample-project/Cargo.toml index a42c678..781eeb9 100644 --- a/sample/cargo-difftests-sample-project/Cargo.toml +++ b/sample/cargo-difftests-sample-project/Cargo.toml @@ -6,7 +6,10 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] -default = ["cargo-difftests-testclient/parallel-groups"] +default = [ + "cargo-difftests-testclient/parallel-groups", + "cargo-difftests-testclient/compile-index-and-clean", +] [dependencies] diff --git a/sample/cargo-difftests-sample-project/tests/tests.rs b/sample/cargo-difftests-sample-project/tests/tests.rs index b118cc1..f44f977 100644 --- a/sample/cargo-difftests-sample-project/tests/tests.rs +++ b/sample/cargo-difftests-sample-project/tests/tests.rs @@ -25,9 +25,9 @@ fn setup_difftests(test_name: &str) -> DifftestsEnv { // the temporary directory where we will store everything we need. // this should be passed to various `cargo difftests` subcommands as the // `--dir` option. - let tmpdir = std::path::PathBuf::from(env!("CARGO_TARGET_TMPDIR")) - .join("cargo-difftests") - .join(test_name); + let root_dir = + std::path::PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("cargo-difftests"); + let tmpdir = root_dir.join(test_name); let difftests_env = cargo_difftests_testclient::init( cargo_difftests_testclient::TestDesc:: { // a "description" of the test. @@ -57,7 +57,15 @@ fn setup_difftests(test_name: &str) -> DifftestsEnv { // pass some environment variables to them, like this: // // cmd.envs(difftests_env.env_for_children()); - difftests_env + difftests_env.and_compile_index_and_clean_on_exit(|b| { + // it also supports the immediate compilation of the index, + // to reduce the total size of profiles on disk. + // (enable the compile-index-and-clean feature) + b.roots(std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("index_root"), root_dir) + .flatten_files_to( + cargo_difftests_testclient::compile_index_and_clean_config::FlattenFilesToTarget::RepoRoot, + ) + }) } }