diff --git a/collector/Cargo.toml b/collector/Cargo.toml index 187a5c512..74ef4c094 100644 --- a/collector/Cargo.toml +++ b/collector/Cargo.toml @@ -9,7 +9,7 @@ rust-version = "1.85.0" [dependencies] anyhow = { workspace = true } chrono = { workspace = true, features = ["serde"] } -clap = { workspace = true, features = ["derive"] } +clap = { workspace = true, features = ["derive", "env"] } env_logger = { workspace = true } hashbrown = { workspace = true } log = { workspace = true } diff --git a/collector/benchlib/src/benchmark.rs b/collector/benchlib/src/benchmark.rs index 670ee50d3..061c7e347 100644 --- a/collector/benchlib/src/benchmark.rs +++ b/collector/benchlib/src/benchmark.rs @@ -58,7 +58,7 @@ impl<'a> BenchmarkGroup<'a> { profile_fn: Box::new(move || profile_function(constructor2.as_ref())), }; if self.benchmarks.insert(name, benchmark_fns).is_some() { - panic!("Benchmark '{}' was registered twice", name); + panic!("Benchmark '{name}' was registered twice"); } } diff --git a/collector/src/bin/collector.rs b/collector/src/bin/collector.rs index 5d9502c95..4e7b00a00 100644 --- a/collector/src/bin/collector.rs +++ b/collector/src/bin/collector.rs @@ -62,7 +62,7 @@ use database::{ArtifactId, ArtifactIdNumber, Commit, CommitType, Connection, Poo fn n_normal_benchmarks_remaining(n: usize) -> String { let suffix = if n == 1 { "" } else { "s" }; - format!("{} normal benchmark{} remaining", n, suffix) + format!("{n} normal benchmark{suffix} remaining") } struct BenchmarkErrors(usize); @@ -160,7 +160,7 @@ fn generate_diffs( vec![format!("{:?}", scenario)] } Scenario::IncrPatched => (0..benchmark.patches.len()) - .map(|i| format!("{:?}{}", scenario, i)) + .map(|i| format!("{scenario:?}{i}")) .collect::>(), } }) { @@ -175,7 +175,7 @@ fn generate_diffs( profiler.postfix() ) }; - let id_diff = format!("{}-{}", id1, id2); + let id_diff = format!("{id1}-{id2}"); let prefix = profiler.prefix(); let left = out_dir.join(filename(prefix, id1)); let right = out_dir.join(filename(prefix, id2)); @@ -183,7 +183,7 @@ fn generate_diffs( if let Err(e) = profiler.diff(&left, &right, &output) { errors.incr(); - eprintln!("collector error: {:?}", e); + eprintln!("collector error: {e:?}"); continue; } @@ -249,7 +249,7 @@ fn main() { match main_result() { Ok(code) => process::exit(code), Err(err) => { - eprintln!("collector error: {:?}", err); + eprintln!("collector error: {err:?}"); process::exit(1); } } @@ -411,7 +411,7 @@ struct DbOption { /// Database output file // This would be better as a `PathBuf`, but it's used in various ways that // make that tricky without adjusting several points in the code. - #[arg(long, default_value = "results.db")] + #[arg(long, default_value = "results.db", env = "DATABASE_URL")] db: String, } @@ -668,20 +668,26 @@ enum Commands { modified: Option, }, - /// Registers a collector in the database + /// Registers a new collector in the database. + /// Use `--is_active` to immediately mark the collector as active. AddCollector { #[command(flatten)] db: DbOption, + /// Name of the collector. #[arg(long)] collector_name: String, + /// Target tuple which will the collector be benchmarking. #[arg(long)] target: String, + /// Should the collector be marked as active immediately? + /// Only active collectors will receive jobs. #[arg(long)] is_active: bool, + /// The benchmark set index that the collector will be benchmarking. #[arg(long)] benchmark_set: u32, }, @@ -766,14 +772,15 @@ fn main_result() -> anyhow::Result { runtime: &runtime_benchmark_dir, }; - // XXX: This doesn't necessarily work for all archs - let target_triple = format!("{}-unknown-linux-gnu", std::env::consts::ARCH); + // This clearly won't work for all architectures, but should be good enough for x64 Linux + // and ARM 64-bit Linux. + let host_target_tuple = format!("{}-unknown-linux-gnu", std::env::consts::ARCH); match args.command { Commands::BinaryStats { mode, symbols } => { match mode { BinaryStatsMode::Compile(args) => { - binary_stats_compile(args, symbols, &target_triple)?; + binary_stats_compile(args, symbols, &host_target_tuple)?; } BinaryStatsMode::Local(args) => { binary_stats_local(args, symbols)?; @@ -792,7 +799,7 @@ fn main_result() -> anyhow::Result { purge, } => { log_db(&db); - let toolchain = get_local_toolchain_for_runtime_benchmarks(&local, &target_triple)?; + let toolchain = get_local_toolchain_for_runtime_benchmarks(&local, &host_target_tuple)?; let pool = Pool::open(&db.db); let isolation_mode = if no_isolate { @@ -846,7 +853,7 @@ fn main_result() -> anyhow::Result { rustc, ToolchainConfig::default(), id, - target_triple.clone(), + host_target_tuple.clone(), )?; let suite = prepare_runtime_benchmark_suite( &toolchain, @@ -903,7 +910,7 @@ fn main_result() -> anyhow::Result { rustc, ToolchainConfig::default(), id, - target_triple.clone(), + host_target_tuple.clone(), )?; Ok::<_, anyhow::Error>(toolchain) }; @@ -946,7 +953,7 @@ fn main_result() -> anyhow::Result { .cargo(local.cargo.as_deref(), local.cargo_config.as_slice()) .id(local.id.as_deref()), "", - target_triple, + host_target_tuple, )?; let mut benchmarks = get_compile_benchmarks(&compile_benchmark_dir, (&local).into())?; @@ -991,7 +998,7 @@ fn main_result() -> anyhow::Result { println!("processing artifacts"); let client = reqwest::blocking::Client::new(); let response: collector::api::next_artifact::Response = client - .get(format!("{}/perf/next_artifact", site_url)) + .get(format!("{site_url}/perf/next_artifact")) .send()? .json()?; let next = if let Some(c) = response.artifact { @@ -1015,7 +1022,7 @@ fn main_result() -> anyhow::Result { match next { NextArtifact::Release(tag) => { let toolchain = - create_toolchain_from_published_version(&tag, &target_triple)?; + create_toolchain_from_published_version(&tag, &host_target_tuple)?; let conn = rt.block_on(pool.connection()); rt.block_on(bench_published_artifact(conn, toolchain, &benchmark_dirs)) } @@ -1055,9 +1062,10 @@ fn main_result() -> anyhow::Result { } }; let sha = commit.sha.to_string(); - let sysroot = Sysroot::install(sha.clone(), &target_triple, &backends) + let sysroot = rt + .block_on(Sysroot::install(sha.clone(), &host_target_tuple, &backends)) .with_context(|| { - format!("failed to install sysroot for {:?}", commit) + format!("failed to install sysroot for {commit:?}") })?; let mut benchmarks = get_compile_benchmarks( @@ -1118,7 +1126,7 @@ fn main_result() -> anyhow::Result { } }); // We need to send a message to this endpoint even if the collector panics - client.post(format!("{}/perf/onpush", site_url)).send()?; + client.post(format!("{site_url}/perf/onpush")).send()?; match res { Ok(res) => res?, @@ -1134,7 +1142,8 @@ fn main_result() -> anyhow::Result { let pool = database::Pool::open(&db.db); let rt = build_async_runtime(); let conn = rt.block_on(pool.connection()); - let toolchain = create_toolchain_from_published_version(&toolchain, &target_triple)?; + let toolchain = + create_toolchain_from_published_version(&toolchain, &host_target_tuple)?; rt.block_on(bench_published_artifact(conn, toolchain, &benchmark_dirs))?; Ok(0) } @@ -1182,7 +1191,7 @@ fn main_result() -> anyhow::Result { .cargo(local.cargo.as_deref(), local.cargo_config.as_slice()) .id(local.id.as_deref()), suffix, - target_triple.clone(), + host_target_tuple.clone(), )?; let id = toolchain.id.clone(); profile_compile( @@ -1245,7 +1254,13 @@ fn main_result() -> anyhow::Result { let last_sha = String::from_utf8(last_sha.stdout).expect("utf8"); let last_sha = last_sha.split_whitespace().next().expect(&last_sha); let commit = get_commit_or_fake_it(last_sha).expect("success"); - let mut sysroot = Sysroot::install(commit.sha, &target_triple, &codegen_backends.0)?; + + let rt = build_async_runtime(); + let mut sysroot = rt.block_on(Sysroot::install( + commit.sha, + &host_target_tuple, + &codegen_backends.0, + ))?; sysroot.preserve(); // don't delete it // Print the directory containing the toolchain. @@ -1313,7 +1328,7 @@ Make sure to modify `{dir}/perf-config.json` if the category/artifact don't matc let target = database::Target::from_str(&target).map_err(|e| anyhow::anyhow!(e))?; rt.block_on(conn.add_collector_config( &collector_name, - &target, + target, benchmark_set, is_active, ))?; @@ -1331,8 +1346,9 @@ Make sure to modify `{dir}/perf-config.json` if the category/artifact don't matc // Obtain the configuration and validate that it matches the // collector's setup - let collector_config: database::CollectorConfig = - rt.block_on(conn.get_collector_config(&collector_name))?; + let collector_config: database::CollectorConfig = rt + .block_on(conn.get_collector_config(&collector_name))? + .unwrap(); let collector_target = collector_config.target(); if collector_target.as_str() != target { @@ -1350,7 +1366,7 @@ Make sure to modify `{dir}/perf-config.json` if the category/artifact don't matc if let Some(benchmark_job) = benchmark_job { // TODO; process the job - println!("{:?}", benchmark_job); + println!("{benchmark_job:?}"); } Ok(0) @@ -1642,7 +1658,7 @@ fn print_binary_stats( .corner_top_right('│'), ), ); - println!("{}", table); + println!("{table}"); } fn get_local_toolchain_for_runtime_benchmarks( @@ -1913,15 +1929,14 @@ async fn bench_compile( let result = measure(&mut processor).await; if let Err(s) = result { eprintln!( - "collector error: Failed to benchmark '{}', recorded: {:#}", - benchmark_name, s + "collector error: Failed to benchmark '{benchmark_name}', recorded: {s:#}" ); errors.incr(); tx.conn() .record_error( collector.artifact_row_id, &benchmark_name.0, - &format!("{:?}", s), + &format!("{s:?}"), ) .await; }; diff --git a/collector/src/bin/rustc-fake.rs b/collector/src/bin/rustc-fake.rs index 387fafa41..8cd4621aa 100644 --- a/collector/src/bin/rustc-fake.rs +++ b/collector/src/bin/rustc-fake.rs @@ -30,8 +30,7 @@ fn run_with_determinism_env(mut cmd: Command) { let status = cmd.status().expect("failed to spawn"); assert!( status.success(), - "command did not complete successfully: {:?}", - cmd + "command did not complete successfully: {cmd:?}" ); } @@ -70,7 +69,7 @@ fn main() { .ok() .and_then(|v| v.parse::().ok()) { - args.push(OsString::from(format!("-Zthreads={}", count))); + args.push(OsString::from(format!("-Zthreads={count}"))); } args.push(OsString::from("-Adeprecated")); @@ -408,7 +407,7 @@ fn main() { } _ => { - panic!("unknown wrapper: {}", wrapper); + panic!("unknown wrapper: {wrapper}"); } } } else if args.iter().any(|arg| arg == "--skip-this-rustc") { @@ -421,7 +420,7 @@ fn main() { .iter() .any(|arg| arg == "-vV" || arg == "--print=file-names") { - eprintln!("{:?} {:?}", tool, args); + eprintln!("{tool:?} {args:?}"); eprintln!("exiting -- non-wrapped rustc"); std::process::exit(1); } @@ -441,7 +440,7 @@ fn process_self_profile_output(prof_out_dir: PathBuf, args: &[OsString]) { .and_then(|args| args[1].to_str()) .expect("rustc to be invoked with crate name"); println!("!self-profile-dir:{}", prof_out_dir.to_str().unwrap()); - println!("!self-profile-crate:{}", crate_name); + println!("!self-profile-crate:{crate_name}"); } #[cfg(windows)] @@ -456,9 +455,9 @@ fn exec(cmd: &mut Command) -> ! { #[cfg(unix)] fn exec(cmd: &mut Command) -> ! { use std::os::unix::prelude::*; - let cmd_d = format!("{:?}", cmd); + let cmd_d = format!("{cmd:?}"); let error = cmd.exec(); - panic!("failed to exec `{}`: {}", cmd_d, error); + panic!("failed to exec `{cmd_d}`: {error}"); } #[cfg(unix)] diff --git a/collector/src/codegen.rs b/collector/src/codegen.rs index b3f27aa13..510f1c1e5 100644 --- a/collector/src/codegen.rs +++ b/collector/src/codegen.rs @@ -203,7 +203,7 @@ fn write_diff(writer: &mut W, use_color: bool, diff: &CodegenDiff) -> style.apply_to(change) )?; } else { - write!(writer, "{}{}", sign, change)?; + write!(writer, "{sign}{change}")?; } } writeln!(writer, "-----------------------------")?; @@ -227,7 +227,7 @@ fn get_codegen( } CodegenType::LLVM | CodegenType::MIR => {} } - cmd.arg(format!("{}", function_index)); + cmd.arg(format!("{function_index}")); Ok(String::from_utf8(command_output(&mut cmd)?.stdout)?) } diff --git a/collector/src/compile/benchmark/codegen_backend.rs b/collector/src/compile/benchmark/codegen_backend.rs index e0f6963b1..6bee4603b 100644 --- a/collector/src/compile/benchmark/codegen_backend.rs +++ b/collector/src/compile/benchmark/codegen_backend.rs @@ -19,3 +19,12 @@ impl From for database::CodegenBackend { } } } + +impl From for CodegenBackend { + fn from(value: database::CodegenBackend) -> Self { + match value { + database::CodegenBackend::Llvm => CodegenBackend::Llvm, + database::CodegenBackend::Cranelift => CodegenBackend::Cranelift, + } + } +} diff --git a/collector/src/compile/benchmark/mod.rs b/collector/src/compile/benchmark/mod.rs index eb653868a..04b03ca07 100644 --- a/collector/src/compile/benchmark/mod.rs +++ b/collector/src/compile/benchmark/mod.rs @@ -141,9 +141,9 @@ impl Benchmark { let config: BenchmarkConfig = if config_path.exists() { serde_json::from_reader( File::open(&config_path) - .with_context(|| format!("failed to open {:?}", config_path))?, + .with_context(|| format!("failed to open {config_path:?}"))?, ) - .with_context(|| format!("failed to parse {:?}", config_path))? + .with_context(|| format!("failed to parse {config_path:?}"))? } else { bail!("missing a perf-config.json file for `{}`", name); }; @@ -207,7 +207,7 @@ impl Benchmark { .ok() .and_then(|v| v.parse::().ok()) { - cargo_args.push(format!("-j{}", count)); + cargo_args.push(format!("-j{count}")); } CargoProcess { @@ -478,7 +478,7 @@ impl Benchmark { // An incremental build with some changes (realistic // incremental case). - let scenario_str = format!("IncrPatched{}", i); + let scenario_str = format!("IncrPatched{i}"); self.mk_cargo_process(toolchain, cwd, profile, backend, target) .incremental(true) .processor( @@ -734,8 +734,8 @@ mod tests { for benchmark in benchmarks { let dir = benchmark_dir.join(&benchmark.name.0); - let cargo_toml = std::fs::read_to_string(&dir.join("Cargo.toml")) - .expect(&format!("Cannot read Cargo.toml of {}", benchmark.name)); + let cargo_toml = std::fs::read_to_string(dir.join("Cargo.toml")) + .unwrap_or_else(|_| panic!("Cannot read Cargo.toml of {}", benchmark.name)); assert!( cargo_toml.contains("[workspace]"), "{} does not contain [workspace] in its Cargo.toml", diff --git a/collector/src/compile/benchmark/target.rs b/collector/src/compile/benchmark/target.rs index 2d84ff0ab..9903c3add 100644 --- a/collector/src/compile/benchmark/target.rs +++ b/collector/src/compile/benchmark/target.rs @@ -22,7 +22,7 @@ impl FromStr for Target { fn from_str(s: &str) -> Result { Ok(match s.to_ascii_lowercase().as_str() { "x86_64-unknown-linux-gnu" => Target::X86_64UnknownLinuxGnu, - _ => return Err(format!("{} is not a valid target", s)), + _ => return Err(format!("{s} is not a valid target")), }) } } diff --git a/collector/src/compile/execute/bencher.rs b/collector/src/compile/execute/bencher.rs index ac19a9ebc..2832ea69e 100644 --- a/collector/src/compile/execute/bencher.rs +++ b/collector/src/compile/execute/bencher.rs @@ -244,7 +244,7 @@ impl Processor for BenchProcessor<'_> { | DeserializeStatError::XperfError(..) | DeserializeStatError::IOError(..)), ) => { - panic!("process_perf_stat_output failed: {:?}", e); + panic!("process_perf_stat_output failed: {e:?}"); } } }) @@ -320,7 +320,7 @@ impl SelfProfileS3Upload { data.read_to_end(&mut compressed).expect("compressed"); std::fs::write(upload.path(), &compressed).expect("write compressed profile data"); - format!("self-profile-{}.mm_profdata.sz", collection) + format!("self-profile-{collection}.mm_profdata.sz") } }; @@ -345,7 +345,7 @@ impl SelfProfileS3Upload { let start = std::time::Instant::now(); let status = self.0.wait().expect("waiting for child"); if !status.success() { - panic!("S3 upload failed: {:?}", status); + panic!("S3 upload failed: {status:?}"); } log::trace!("uploaded to S3, additional wait: {:?}", start.elapsed()); diff --git a/collector/src/compile/execute/etw_parser.rs b/collector/src/compile/execute/etw_parser.rs index d611f93f2..ebe0962d2 100644 --- a/collector/src/compile/execute/etw_parser.rs +++ b/collector/src/compile/execute/etw_parser.rs @@ -169,7 +169,7 @@ fn parse_events(r: &mut dyn BufRead, headers: Vec) -> anyhow::Resul .columns .iter() .position(|c| c == name) - .unwrap_or_else(|| panic!("failed to find column {}", name)) + .unwrap_or_else(|| panic!("failed to find column {name}")) + 1 }; @@ -222,7 +222,7 @@ fn parse_events(r: &mut dyn BufRead, headers: Vec) -> anyhow::Resul let pid = process_name[l_paren + 1..r_paran].trim(); pid.parse() - .unwrap_or_else(|_| panic!("failed to parse '{}' to pid", pid)) + .unwrap_or_else(|_| panic!("failed to parse '{pid}' to pid")) } let mut buffer = Vec::new(); diff --git a/collector/src/compile/execute/mod.rs b/collector/src/compile/execute/mod.rs index 87221a35e..4b5b8a56f 100644 --- a/collector/src/compile/execute/mod.rs +++ b/collector/src/compile/execute/mod.rs @@ -38,8 +38,8 @@ pub enum PerfTool { impl PerfTool { fn name(&self) -> String { match self { - PerfTool::BenchTool(b) => format!("{:?}", b), - PerfTool::ProfileTool(p) => format!("{:?}", p), + PerfTool::BenchTool(b) => format!("{b:?}"), + PerfTool::ProfileTool(p) => format!("{p:?}"), } } @@ -265,7 +265,7 @@ impl<'a> CargoProcess<'a> { } let out = command_output(&mut pkgid_cmd) - .with_context(|| format!("failed to obtain pkgid in '{:?}'", cwd))? + .with_context(|| format!("failed to obtain pkgid in '{cwd:?}'"))? .stdout; let package_id = str::from_utf8(&out).unwrap(); Ok(package_id.trim().to_string()) @@ -671,8 +671,7 @@ fn process_stat_output( } if !pct.starts_with("100.") { panic!( - "measurement of `{}` only active for {}% of the time", - name, pct + "measurement of `{name}` only active for {pct}% of the time" ); } stats.insert( diff --git a/collector/src/compile/execute/profiler.rs b/collector/src/compile/execute/profiler.rs index 069f6037d..23b9d0bfd 100644 --- a/collector/src/compile/execute/profiler.rs +++ b/collector/src/compile/execute/profiler.rs @@ -173,7 +173,7 @@ impl Processor for ProfileProcessor<'_> { } else if filename_str.ends_with(".mm_profdata") { utils::fs::rename(path, filepath(&zsp_dir, "Zsp.mm_profdata"))?; } else { - panic!("unexpected file {:?}", path); + panic!("unexpected file {path:?}"); } } assert!(num_files == 3 || num_files == 1); @@ -357,7 +357,7 @@ impl Processor for ProfileProcessor<'_> { continue; } - writeln!(&mut final_file, "{}", line)?; + writeln!(&mut final_file, "{line}")?; } } @@ -399,10 +399,10 @@ impl Processor for ProfileProcessor<'_> { let cgu_file = filepath(&out_dir, cgu); let mut file = io::BufWriter::new( fs::File::create(&cgu_file) - .with_context(|| format!("{:?}", cgu_file))?, + .with_context(|| format!("{cgu_file:?}"))?, ); for (name, linkage) in items { - writeln!(&mut file, "{} {}", name, linkage)?; + writeln!(&mut file, "{name} {linkage}")?; } } } diff --git a/collector/src/lib.rs b/collector/src/lib.rs index 49dbb937d..3ca21ff48 100644 --- a/collector/src/lib.rs +++ b/collector/src/lib.rs @@ -176,7 +176,7 @@ fn run_command_with_output(cmd: &mut Command) -> anyhow::Result .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .with_context(|| format!("failed to spawn process for cmd: {:?}", cmd))?; + .with_context(|| format!("failed to spawn process for cmd: {cmd:?}"))?; let mut stdout = Vec::new(); let mut stderr = Vec::new(); @@ -239,7 +239,7 @@ pub async fn async_command_output( .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .with_context(|| format!("failed to spawn process for cmd: {:?}", cmd))?; + .with_context(|| format!("failed to spawn process for cmd: {cmd:?}"))?; let output = child.wait_with_output().await?; log::trace!("command {cmd:?} took {} ms", start.elapsed().as_millis()); @@ -385,5 +385,5 @@ impl CollectorCtx { } pub fn runtime_group_step_name(benchmark_name: &str) -> String { - format!("runtime:{}", benchmark_name) + format!("runtime:{benchmark_name}") } diff --git a/collector/src/runtime/mod.rs b/collector/src/runtime/mod.rs index a18d04afe..a5baaa262 100644 --- a/collector/src/runtime/mod.rs +++ b/collector/src/runtime/mod.rs @@ -37,7 +37,7 @@ pub async fn bench_runtime( iterations: u32, ) -> anyhow::Result<()> { let filtered = suite.filtered_benchmark_count(&filter); - println!("Executing {} benchmarks\n", filtered); + println!("Executing {filtered} benchmarks\n"); let rustc_perf_version = get_rustc_perf_commit(); let mut benchmark_index = 0; @@ -87,12 +87,12 @@ pub async fn bench_runtime( .with_context(|| format!("Failed to execute runtime benchmark group {}", group.name)); if let Err(error) = result { - eprintln!("collector error: {:#}", error); + eprintln!("collector error: {error:#}"); tx.conn() .record_error( collector.artifact_row_id, &step_name, - &format!("{:?}", error), + &format!("{error:?}"), ) .await; }; diff --git a/collector/src/toolchain.rs b/collector/src/toolchain.rs index 82ee3e808..6d9e11812 100644 --- a/collector/src/toolchain.rs +++ b/collector/src/toolchain.rs @@ -20,7 +20,11 @@ pub struct Sysroot { } impl Sysroot { - pub fn install(sha: String, triple: &str, backends: &[CodegenBackend]) -> anyhow::Result { + pub async fn install( + sha: String, + triple: &str, + backends: &[CodegenBackend], + ) -> anyhow::Result { let unpack_into = "cache"; fs::create_dir_all(unpack_into)?; @@ -31,12 +35,12 @@ impl Sysroot { triple: triple.to_owned(), }; - download.get_and_extract(Component::Rustc)?; - download.get_and_extract(Component::Std)?; - download.get_and_extract(Component::Cargo)?; - download.get_and_extract(Component::RustSrc)?; + download.get_and_extract(Component::Rustc).await?; + download.get_and_extract(Component::Std).await?; + download.get_and_extract(Component::Cargo).await?; + download.get_and_extract(Component::RustSrc).await?; if backends.contains(&CodegenBackend::Cranelift) { - download.get_and_extract(Component::Cranelift)?; + download.get_and_extract(Component::Cranelift).await?; } let sysroot = download.into_sysroot()?; @@ -99,7 +103,7 @@ impl Component { let suffix = if *self == Component::RustSrc { String::new() } else { - format!("-{}", triple) + format!("-{triple}") }; format!( "{base}/{sha}/{module}-{channel}{suffix}.tar.xz", @@ -141,7 +145,7 @@ impl SysrootDownload { }) } - fn get_and_extract(&self, component: Component) -> anyhow::Result<()> { + async fn get_and_extract(&self, component: Component) -> anyhow::Result<()> { let archive_path = self.directory.join(format!( "{}-{}-{}.tar.xz", self.rust_sha, self.triple, component, @@ -168,10 +172,11 @@ impl SysrootDownload { ]; for url in &urls { log::debug!("requesting: {}", url); - let resp = reqwest::blocking::get(url)?; + let resp = reqwest::get(url).await?; log::debug!("{}", resp.status()); if resp.status().is_success() { - let reader = XzDecoder::new(BufReader::new(resp)); + let bytes: Vec = resp.bytes().await?.into(); + let reader = XzDecoder::new(BufReader::new(bytes.as_slice())); match self.extract(component, reader) { Ok(()) => return Ok(()), Err(err) => { @@ -441,7 +446,7 @@ pub fn get_local_toolchain( } else { let rustc = PathBuf::from(rustc) .canonicalize() - .with_context(|| format!("failed to canonicalize rustc executable {:?}", rustc))?; + .with_context(|| format!("failed to canonicalize rustc executable {rustc:?}"))?; // When specifying rustc via a path, the suffix is always added to the // id. @@ -458,7 +463,7 @@ pub fn get_local_toolchain( let rustdoc = if let Some(rustdoc) = &toolchain_config.rustdoc { Some(rustdoc.canonicalize().with_context(|| { - format!("failed to canonicalize rustdoc executable {:?}", rustdoc) + format!("failed to canonicalize rustdoc executable {rustdoc:?}") })?) } else if profiles.iter().any(|p| p.is_doc()) { // We need a `rustdoc`. Look for one next to `rustc`. @@ -479,7 +484,7 @@ pub fn get_local_toolchain( let clippy = if let Some(clippy) = &toolchain_config.clippy { Some( clippy.canonicalize().with_context(|| { - format!("failed to canonicalize clippy executable {:?}", clippy) + format!("failed to canonicalize clippy executable {clippy:?}") })?, ) } else if profiles.contains(&Profile::Clippy) { @@ -500,7 +505,7 @@ pub fn get_local_toolchain( let cargo = if let Some(cargo) = &toolchain_config.cargo { cargo .canonicalize() - .with_context(|| format!("failed to canonicalize cargo executable {:?}", cargo))? + .with_context(|| format!("failed to canonicalize cargo executable {cargo:?}"))? } else { // Use the nightly cargo from `rustup`. let output = Command::new("rustup") diff --git a/collector/src/utils/cachegrind.rs b/collector/src/utils/cachegrind.rs index f47d40aad..f3414c54d 100644 --- a/collector/src/utils/cachegrind.rs +++ b/collector/src/utils/cachegrind.rs @@ -54,7 +54,7 @@ pub fn cachegrind_annotate( continue; } } - writeln!(writer, "{}", line)?; + writeln!(writer, "{line}")?; } writer.flush()?; diff --git a/collector/src/utils/fs.rs b/collector/src/utils/fs.rs index e38176df4..dd2e3e24c 100644 --- a/collector/src/utils/fs.rs +++ b/collector/src/utils/fs.rs @@ -29,7 +29,7 @@ pub fn rename, Q: AsRef>(from: P, to: Q) -> anyhow::Result< // mount points (e.g., if /tmp is in tmpfs instead of on // the same disk). We don't want to implement a full recursive solution // to copying directories, so just shell out to `mv`. - let ctx = format!("mv {:?} {:?}", from, to); + let ctx = format!("mv {from:?} {to:?}"); let status = Command::new("mv") .arg(from) .arg(to) @@ -47,7 +47,7 @@ pub fn rename, Q: AsRef>(from: P, to: Q) -> anyhow::Result< pub fn touch(path: &Path) -> anyhow::Result<()> { let file = File::options().read(true).write(true).open(path)?; file.set_modified(SystemTime::now()) - .with_context(|| format!("touching file {:?}", path))?; + .with_context(|| format!("touching file {path:?}"))?; Ok(()) } @@ -80,7 +80,7 @@ pub fn touch_all(path: &Path) -> anyhow::Result<()> { // 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))?; + .with_context(|| format!("deleting cmake caches in {path:?}"))?; } if is_valid(path) { diff --git a/database/schema.md b/database/schema.md index 309e5b74f..1e8fce8ee 100644 --- a/database/schema.md +++ b/database/schema.md @@ -284,6 +284,8 @@ Columns: * **parent_sha** (`text`): Parent SHA of the benchmarked commit. * Can be `NULL` for try requests without compiler artifacts. * **commit_type** (`text NOT NULL`): One of `master`, `try` or `release`. +* **commit_date** (`timestamptz`): Datetime when the compiler artifact commit (not the request) was created. + * Can be `NULL` for try requests without compiler artifacts. * **pr** (`int`): Pull request number associated with the master/try commit. * `NULL` for release requests. * **created_at** (`timestamptz NOT NULL`): Datetime when the request was created. diff --git a/database/src/bin/import-sqlite.rs b/database/src/bin/import-sqlite.rs index 5159d4dae..8b47e4992 100644 --- a/database/src/bin/import-sqlite.rs +++ b/database/src/bin/import-sqlite.rs @@ -21,7 +21,7 @@ async fn main() { let sqlite_idx = sqlite_conn.load_index().await; let cid_name = format!("imported-{}", chrono::Utc::now().timestamp()); - println!("Collection ID for import is {}", cid_name); + println!("Collection ID for import is {cid_name}"); let cid = postgres_conn.collection_id(&cid_name).await; let mut benchmarks = HashSet::new(); @@ -39,7 +39,7 @@ async fn main() { .artifact_by_name(&artifact) .await .unwrap_or_else(|| { - panic!("{} not found in sqlite db", artifact); + panic!("{artifact} not found in sqlite db"); }); let sqlite_aid = sqlite_conn.artifact_id(&aid).await; diff --git a/database/src/bin/sqlite-to-postgres.rs b/database/src/bin/sqlite-to-postgres.rs index 9ac7fb974..138c3fe35 100644 --- a/database/src/bin/sqlite-to-postgres.rs +++ b/database/src/bin/sqlite-to-postgres.rs @@ -780,8 +780,7 @@ async fn copy( let copy = postgres .prepare(&format!( - r#"copy {} ({}) from stdin (encoding utf8, format csv, null '{}')"#, - table, attributes, NULL_STRING, + r#"copy {table} ({attributes}) from stdin (encoding utf8, format csv, null '{NULL_STRING}')"#, )) .await .unwrap(); @@ -862,9 +861,8 @@ async fn copy( &format!( "select setval( pg_get_serial_sequence($1, $2), - coalesce(max({}) + 1, 1), false) - from {}", - generated_id_attr, table + coalesce(max({generated_id_attr}) + 1, 1), false) + from {table}" ) as &str, &[&table, &generated_id_attr], ) @@ -944,7 +942,7 @@ async fn get_tables(postgres: &tokio_postgres::Transaction<'_>) -> Vec { async fn disable_table_triggers(postgres: &tokio_postgres::Transaction<'_>, tables: &[String]) { for table in tables { postgres - .execute(&format!("ALTER TABLE {} DISABLE TRIGGER ALL", table), &[]) + .execute(&format!("ALTER TABLE {table} DISABLE TRIGGER ALL"), &[]) .await .unwrap(); } @@ -954,7 +952,7 @@ async fn disable_table_triggers(postgres: &tokio_postgres::Transaction<'_>, tabl async fn enable_table_triggers(postgres: &tokio_postgres::Transaction<'_>, tables: &[String]) { for table in tables { postgres - .execute(&format!("ALTER TABLE {} ENABLE TRIGGER ALL", table), &[]) + .execute(&format!("ALTER TABLE {table} ENABLE TRIGGER ALL"), &[]) .await .unwrap(); } diff --git a/database/src/lib.rs b/database/src/lib.rs index a8f39c28f..37de4bded 100644 --- a/database/src/lib.rs +++ b/database/src/lib.rs @@ -148,7 +148,7 @@ impl FromStr for CommitType { match ty { "try" => Ok(CommitType::Try), "master" => Ok(CommitType::Master), - _ => Err(format!("Wrong commit type {}", ty)), + _ => Err(format!("Wrong commit type {ty}")), } } } @@ -244,7 +244,7 @@ impl std::str::FromStr for Profile { "doc-json" => Profile::DocJson, "opt" => Profile::Opt, "clippy" => Profile::Clippy, - _ => return Err(format!("{} is not a profile", s)), + _ => return Err(format!("{s} is not a profile")), }) } } @@ -285,7 +285,7 @@ impl std::str::FromStr for Scenario { if let Some(stripped) = s.strip_prefix("incr-patched: ") { Scenario::IncrementalPatch(PatchName::from(stripped)) } else { - return Err(format!("{} is not a scenario", s)); + return Err(format!("{s} is not a scenario")); } } }) @@ -298,7 +298,7 @@ impl fmt::Display for Scenario { Scenario::Empty => write!(f, "full"), Scenario::IncrementalEmpty => write!(f, "incr-full"), Scenario::IncrementalFresh => write!(f, "incr-unchanged"), - Scenario::IncrementalPatch(name) => write!(f, "incr-patched: {}", name), + Scenario::IncrementalPatch(name) => write!(f, "incr-patched: {name}"), } } } @@ -309,7 +309,7 @@ impl Scenario { Scenario::Empty => "full".to_string(), Scenario::IncrementalEmpty => "incr-full".to_string(), Scenario::IncrementalFresh => "incr-unchanged".to_string(), - Scenario::IncrementalPatch(name) => format!("incr-patched-{}", name), + Scenario::IncrementalPatch(name) => format!("incr-patched-{name}"), } } } @@ -381,7 +381,7 @@ impl FromStr for Target { fn from_str(s: &str) -> Result { Ok(match s.to_ascii_lowercase().as_str() { "x86_64-unknown-linux-gnu" => Target::X86_64UnknownLinuxGnu, - _ => return Err(format!("{} is not a valid target", s)), + _ => return Err(format!("{s} is not a valid target")), }) } } @@ -418,7 +418,7 @@ impl FromStr for CodegenBackend { Ok(match s.to_ascii_lowercase().as_str() { "llvm" => CodegenBackend::Llvm, "cranelift" => CodegenBackend::Cranelift, - _ => return Err(format!("{} is not a codegen backend", s)), + _ => return Err(format!("{s} is not a codegen backend")), }) } } @@ -442,7 +442,7 @@ impl fmt::Display for ArtifactId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { ArtifactId::Commit(c) => write!(f, "{} ({})", c.sha, c.date), - ArtifactId::Tag(id) => write!(f, "{}", id), + ArtifactId::Tag(id) => write!(f, "{id}"), } } } @@ -889,6 +889,8 @@ impl fmt::Display for BenchmarkRequestType { #[derive(Debug, Clone, PartialEq)] pub struct BenchmarkRequest { commit_type: BenchmarkRequestType, + // When was the compiler artifact created + commit_date: Option>, created_at: DateTime, status: BenchmarkRequestStatus, backends: String, @@ -897,12 +899,13 @@ pub struct BenchmarkRequest { impl BenchmarkRequest { /// Create a release benchmark request that is in the `ArtifactsReady` status. - pub fn create_release(tag: &str, created_at: DateTime) -> Self { + pub fn create_release(tag: &str, commit_date: DateTime) -> Self { Self { commit_type: BenchmarkRequestType::Release { tag: tag.to_string(), }, - created_at, + commit_date: Some(commit_date), + created_at: Utc::now(), status: BenchmarkRequestStatus::ArtifactsReady, backends: String::new(), profiles: String::new(), @@ -910,19 +913,15 @@ impl BenchmarkRequest { } /// Create a try request that is in the `WaitingForArtifacts` status. - pub fn create_try_without_artifacts( - pr: u32, - created_at: DateTime, - backends: &str, - profiles: &str, - ) -> Self { + pub fn create_try_without_artifacts(pr: u32, backends: &str, profiles: &str) -> Self { Self { commit_type: BenchmarkRequestType::Try { pr, sha: None, parent_sha: None, }, - created_at, + commit_date: None, + created_at: Utc::now(), status: BenchmarkRequestStatus::WaitingForArtifacts, backends: backends.to_string(), profiles: profiles.to_string(), @@ -930,14 +929,15 @@ impl BenchmarkRequest { } /// Create a master benchmark request that is in the `ArtifactsReady` status. - pub fn create_master(sha: &str, parent_sha: &str, pr: u32, created_at: DateTime) -> Self { + pub fn create_master(sha: &str, parent_sha: &str, pr: u32, commit_date: DateTime) -> Self { Self { commit_type: BenchmarkRequestType::Master { pr, sha: sha.to_string(), parent_sha: parent_sha.to_string(), }, - created_at, + commit_date: Some(commit_date), + created_at: Utc::now(), status: BenchmarkRequestStatus::ArtifactsReady, backends: String::new(), profiles: String::new(), @@ -1093,8 +1093,14 @@ impl fmt::Display for BenchmarkJobStatus { } } -#[derive(Debug, Clone, PartialEq)] -pub struct BenchmarkSet(pub u32); +#[derive(Debug, Copy, Clone, PartialEq)] +pub struct BenchmarkSet(u32); + +impl BenchmarkSet { + pub fn new(id: u32) -> Self { + Self(id) + } +} /// A single unit of work generated from a benchmark request. Split by profiles /// and backends @@ -1112,7 +1118,7 @@ pub struct BenchmarkJob { benchmark_set: BenchmarkSet, created_at: DateTime, status: BenchmarkJobStatus, - retry: u32, + deque_counter: u32, } impl BenchmarkJob { @@ -1120,24 +1126,24 @@ impl BenchmarkJob { self.id } - pub fn target(&self) -> &Target { - &self.target + pub fn target(&self) -> Target { + self.target } - pub fn backend(&self) -> &CodegenBackend { - &self.backend + pub fn backend(&self) -> CodegenBackend { + self.backend } - pub fn profile(&self) -> &Profile { - &self.profile + pub fn profile(&self) -> Profile { + self.profile } pub fn request_tag(&self) -> &str { &self.request_tag } - pub fn benchmark_set(&self) -> &BenchmarkSet { - &self.benchmark_set + pub fn benchmark_set(&self) -> BenchmarkSet { + self.benchmark_set } pub fn collector_name(&self) -> Option<&str> { @@ -1147,6 +1153,11 @@ impl BenchmarkJob { | BenchmarkJobStatus::Completed { collector_name, .. } => Some(collector_name), } } + + /// How many times was the job already dequed? + pub fn deque_count(&self) -> u32 { + self.deque_counter + } } /// Describes the final state of a job @@ -1181,12 +1192,12 @@ impl CollectorConfig { &self.name } - pub fn target(&self) -> &Target { - &self.target + pub fn target(&self) -> Target { + self.target } - pub fn benchmark_set(&self) -> &BenchmarkSet { - &self.benchmark_set + pub fn benchmark_set(&self) -> BenchmarkSet { + self.benchmark_set } pub fn is_active(&self) -> bool { diff --git a/database/src/pool.rs b/database/src/pool.rs index dd0571045..b55246ddf 100644 --- a/database/src/pool.rs +++ b/database/src/pool.rs @@ -211,15 +211,16 @@ pub trait Connection: Send + Sync { pr: u32, sha: &str, parent_sha: &str, + commit_date: DateTime, ) -> anyhow::Result<()>; /// Add a benchmark job to the job queue. async fn enqueue_benchmark_job( &self, request_tag: &str, - target: &Target, - backend: &CodegenBackend, - profile: &Profile, + target: Target, + backend: CodegenBackend, + profile: Profile, benchmark_set: u32, ) -> anyhow::Result<()>; @@ -238,21 +239,26 @@ pub trait Connection: Send + Sync { async fn add_collector_config( &self, collector_name: &str, - target: &Target, + target: Target, benchmark_set: u32, is_active: bool, ) -> anyhow::Result; - /// Get the confiuguration for a collector by the name of the collector - async fn get_collector_config(&self, collector_name: &str) -> anyhow::Result; + /// Get the configuration for a collector by its name. + async fn get_collector_config( + &self, + collector_name: &str, + ) -> anyhow::Result>; - /// Dequeue benchmark job + /// Dequeues a single job for the given collector, target and benchmark set. + /// Also returns detailed information about the compiler artifact that should be benchmarked + /// in the job. async fn dequeue_benchmark_job( &self, collector_name: &str, - target: &Target, - benchmark_set: &BenchmarkSet, - ) -> anyhow::Result>; + target: Target, + benchmark_set: BenchmarkSet, + ) -> anyhow::Result>; /// Try and mark the benchmark_request as completed. Will return `true` if /// it has been marked as completed else `false` meaning there was no change @@ -263,7 +269,7 @@ pub trait Connection: Send + Sync { async fn mark_benchmark_job_as_completed( &self, id: u32, - benchmark_job_conculsion: &BenchmarkJobConclusion, + benchmark_job_conculsion: BenchmarkJobConclusion, ) -> anyhow::Result<()>; } @@ -405,21 +411,21 @@ mod tests { request_tag: &str, collector_name: &str, benchmark_set: u32, - target: &Target, + target: Target, ) { /* Create job for the request */ db.enqueue_benchmark_job( request_tag, - &target, - &CodegenBackend::Llvm, - &Profile::Opt, + target, + CodegenBackend::Llvm, + Profile::Opt, benchmark_set, ) .await .unwrap(); - let job = db - .dequeue_benchmark_job(collector_name, &target, &BenchmarkSet(benchmark_set)) + let (job, _) = db + .dequeue_benchmark_job(collector_name, target, BenchmarkSet(benchmark_set)) .await .unwrap() .unwrap(); @@ -427,15 +433,14 @@ mod tests { assert_eq!(job.request_tag(), request_tag); /* Mark the job as complete */ - db.mark_benchmark_job_as_completed(job.id(), &BenchmarkJobConclusion::Success) + db.mark_benchmark_job_as_completed(job.id(), BenchmarkJobConclusion::Success) .await .unwrap(); - assert_eq!( + assert!( db.mark_benchmark_request_as_completed(request_tag) .await - .unwrap(), - true + .unwrap() ); } @@ -446,7 +451,7 @@ mod tests { // wired up correctly. Though makes sense that there should be // an empty vector returned if there are no pstats. let db = ctx.db_client(); - let result = db.connection().await.get_pstats(&vec![], &vec![]).await; + let result = db.connection().await.get_pstats(&[], &[]).await; let expected: Vec>> = vec![]; assert_eq!(result, expected); @@ -527,30 +532,30 @@ mod tests { async fn multiple_non_completed_try_requests() { run_postgres_test(|ctx| async { let db = ctx.db_client().connection().await; - let target = &Target::X86_64UnknownLinuxGnu; + let target = Target::X86_64UnknownLinuxGnu; let collector_name = "collector-1"; let benchmark_set = 1; - db.add_collector_config(collector_name, &target, benchmark_set, true) + db.add_collector_config(collector_name, target, benchmark_set, true) .await .unwrap(); // Complete parent let parent = BenchmarkRequest::create_release("sha-parent-1", Utc::now()); // Complete - let req_a = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", ""); + let req_a = BenchmarkRequest::create_try_without_artifacts(42, "", ""); // WaitingForArtifacts - let req_b = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", ""); - let req_c = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", ""); + let req_b = BenchmarkRequest::create_try_without_artifacts(42, "", ""); + let req_c = BenchmarkRequest::create_try_without_artifacts(42, "", ""); db.insert_benchmark_request(&parent).await.unwrap(); db.insert_benchmark_request(&req_a).await.unwrap(); - db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1") + db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1", Utc::now()) .await .unwrap(); - complete_request(&*db, "sha-parent-1", collector_name, benchmark_set, &target).await; - complete_request(&*db, "sha1", collector_name, benchmark_set, &target).await; + complete_request(&*db, "sha-parent-1", collector_name, benchmark_set, target).await; + complete_request(&*db, "sha1", collector_name, benchmark_set, target).await; // This should be fine, req_a was completed db.insert_benchmark_request(&req_b).await.unwrap(); @@ -599,11 +604,11 @@ mod tests { run_postgres_test(|ctx| async { let db = ctx.db_client().connection().await; let time = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap(); - let target = &Target::X86_64UnknownLinuxGnu; + let target = Target::X86_64UnknownLinuxGnu; let collector_name = "collector-1"; let benchmark_set = 1; - db.add_collector_config(collector_name, &target, benchmark_set, true) + db.add_collector_config(collector_name, target, benchmark_set, true) .await .unwrap(); @@ -612,7 +617,7 @@ mod tests { // ArtifactsReady let req_b = BenchmarkRequest::create_release("1.80.0", time); // WaitingForArtifacts - let req_c = BenchmarkRequest::create_try_without_artifacts(50, time, "", ""); + let req_c = BenchmarkRequest::create_try_without_artifacts(50, "", ""); // InProgress let req_d = BenchmarkRequest::create_master("sha-2", "parent-sha-2", 51, time); // Completed @@ -622,7 +627,7 @@ mod tests { db.insert_benchmark_request(req).await.unwrap(); } - complete_request(&*db, "1.79.0", collector_name, benchmark_set, &target).await; + complete_request(&*db, "1.79.0", collector_name, benchmark_set, target).await; db.update_benchmark_request_status("sha-2", BenchmarkRequestStatus::InProgress) .await @@ -646,10 +651,10 @@ mod tests { let db = ctx.db_client(); let db = db.connection().await; - let req = BenchmarkRequest::create_try_without_artifacts(42, Utc::now(), "", ""); + let req = BenchmarkRequest::create_try_without_artifacts(42, "", ""); db.insert_benchmark_request(&req).await.unwrap(); - db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1") + db.attach_shas_to_try_benchmark_request(42, "sha1", "sha-parent-1", Utc::now()) .await .unwrap(); @@ -671,8 +676,8 @@ mod tests { BenchmarkRequestType::Try { .. } )); - assert_eq!(req_db.tag().as_deref(), Some("sha1")); - assert_eq!(req_db.parent_sha().as_deref(), Some("sha-parent-1")); + assert_eq!(req_db.tag(), Some("sha1")); + assert_eq!(req_db.parent_sha(), Some("sha-parent-1")); assert_eq!(req_db.pr(), Some(&42)); Ok(ctx) @@ -698,9 +703,9 @@ mod tests { let result = db .enqueue_benchmark_job( benchmark_request.tag().unwrap(), - &Target::X86_64UnknownLinuxGnu, - &CodegenBackend::Llvm, - &Profile::Opt, + Target::X86_64UnknownLinuxGnu, + CodegenBackend::Llvm, + Profile::Opt, 0u32, ) .await; @@ -775,9 +780,9 @@ mod tests { run_postgres_test(|ctx| async { let db = ctx.db_client().connection().await; - let collector_config_result = db.get_collector_config("collector-1").await; + let collector_config_result = db.get_collector_config("collector-1").await.unwrap(); - assert!(collector_config_result.is_err()); + assert!(collector_config_result.is_none()); Ok(ctx) }) @@ -789,17 +794,20 @@ mod tests { run_postgres_test(|ctx| async { let db = ctx.db_client().connection().await; - let insert_config_result = db - .add_collector_config("collector-1", &Target::X86_64UnknownLinuxGnu, 1, true) - .await; - assert!(insert_config_result.is_ok()); + let inserted_config = db + .add_collector_config("collector-1", Target::X86_64UnknownLinuxGnu, 1, true) + .await + .unwrap(); - let get_config_result = db.get_collector_config("collector-1").await; - assert!(get_config_result.is_ok()); + let config = db + .get_collector_config("collector-1") + .await + .unwrap() + .expect("collector config not found"); // What we entered into the database should be identical to what is // returned from the database - assert_eq!(insert_config_result.unwrap(), get_config_result.unwrap()); + assert_eq!(inserted_config, config); Ok(ctx) }) .await; @@ -813,8 +821,8 @@ mod tests { let benchmark_job_result = db .dequeue_benchmark_job( "collector-1", - &Target::X86_64UnknownLinuxGnu, - &BenchmarkSet(420), + Target::X86_64UnknownLinuxGnu, + BenchmarkSet(420), ) .await; @@ -832,12 +840,10 @@ mod tests { let db = ctx.db_client().connection().await; let time = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap(); - let insert_result = db - .add_collector_config("collector-1", &Target::X86_64UnknownLinuxGnu, 1, true) - .await; - assert!(insert_result.is_ok()); - - let collector_config = insert_result.unwrap(); + let collector_config = db + .add_collector_config("collector-1", Target::X86_64UnknownLinuxGnu, 1, true) + .await + .unwrap(); let benchmark_request = BenchmarkRequest::create_master("sha-1", "parent-sha-1", 42, time); @@ -848,32 +854,28 @@ mod tests { .unwrap(); // Now we can insert the job - let enqueue_result = db - .enqueue_benchmark_job( - benchmark_request.tag().unwrap(), - &Target::X86_64UnknownLinuxGnu, - &CodegenBackend::Llvm, - &Profile::Opt, - 1u32, - ) - .await; - assert!(enqueue_result.is_ok()); + db.enqueue_benchmark_job( + benchmark_request.tag().unwrap(), + Target::X86_64UnknownLinuxGnu, + CodegenBackend::Llvm, + Profile::Opt, + 1u32, + ) + .await + .unwrap(); - let benchmark_job = db + let (benchmark_job, artifact_id) = db .dequeue_benchmark_job( collector_config.name(), collector_config.target(), collector_config.benchmark_set(), ) - .await; - assert!(benchmark_job.is_ok()); - - let benchmark_job = benchmark_job.unwrap(); - assert!(benchmark_job.is_some()); + .await + .unwrap() + .unwrap(); // Ensure the properties of the job match both the request and the // collector configuration - let benchmark_job = benchmark_job.unwrap(); assert_eq!( benchmark_job.request_tag(), benchmark_request.tag().unwrap() @@ -887,6 +889,15 @@ mod tests { collector_config.name(), ); + assert_eq!( + artifact_id, + ArtifactId::Commit(Commit { + sha: "sha-1".to_string(), + date: Date(time), + r#type: CommitType::Master, + }) + ); + Ok(ctx) }) .await; @@ -899,7 +910,7 @@ mod tests { let time = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap(); let insert_result = db - .add_collector_config("collector-1", &Target::X86_64UnknownLinuxGnu, 1, true) + .add_collector_config("collector-1", Target::X86_64UnknownLinuxGnu, 1, true) .await; assert!(insert_result.is_ok()); @@ -908,11 +919,10 @@ mod tests { db.insert_benchmark_request(&benchmark_request) .await .unwrap(); - assert_eq!( + assert!( db.mark_benchmark_request_as_completed("sha-1") .await - .unwrap(), - true + .unwrap() ); Ok(ctx) }) @@ -930,7 +940,7 @@ mod tests { let target = Target::X86_64UnknownLinuxGnu; let insert_result = db - .add_collector_config(collector_name, &target, 1, true) + .add_collector_config(collector_name, target, 1, true) .await; assert!(insert_result.is_ok()); @@ -943,16 +953,16 @@ mod tests { /* Create job for the request */ db.enqueue_benchmark_job( benchmark_request.tag().unwrap(), - &target, - &CodegenBackend::Llvm, - &Profile::Opt, + target, + CodegenBackend::Llvm, + Profile::Opt, benchmark_set.0, ) .await .unwrap(); - let job = db - .dequeue_benchmark_job(collector_name, &target, &benchmark_set) + let (job, _) = db + .dequeue_benchmark_job(collector_name, target, benchmark_set) .await .unwrap() .unwrap(); @@ -960,7 +970,7 @@ mod tests { assert_eq!(job.request_tag(), benchmark_request.tag().unwrap()); /* Mark the job as complete */ - db.mark_benchmark_job_as_completed(job.id(), &BenchmarkJobConclusion::Success) + db.mark_benchmark_job_as_completed(job.id(), BenchmarkJobConclusion::Success) .await .unwrap(); diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs index 9483a6239..0d103e9ae 100644 --- a/database/src/pool/postgres.rs +++ b/database/src/pool/postgres.rs @@ -51,7 +51,7 @@ pub async fn make_client(db_url: &str) -> anyhow::Result }; tokio::spawn(async move { if let Err(e) = connection.await { - eprintln!("database connection error: {}", e); + eprintln!("database connection error: {e}"); } }); @@ -67,7 +67,7 @@ pub async fn make_client(db_url: &str) -> anyhow::Result }; tokio::spawn(async move { if let Err(e) = connection.await { - eprintln!("database connection error: {}", e); + eprintln!("database connection error: {e}"); } }); @@ -378,6 +378,9 @@ static MIGRATIONS: &[&str] = &[ ALTER TABLE job_queue DROP COLUMN IF EXISTS collector_id; CREATE INDEX IF NOT EXISTS job_queue_status_target_benchmark_set_idx ON job_queue (status, target, benchmark_set); "#, + r#" + ALTER TABLE benchmark_request ADD COLUMN commit_date TIMESTAMPTZ NULL; + "#, ]; #[async_trait::async_trait] @@ -1485,9 +1488,10 @@ where status, created_at, backends, - profiles + profiles, + commit_date ) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8); + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9); "#, &[ &benchmark_request.tag(), @@ -1498,6 +1502,7 @@ where &benchmark_request.created_at, &benchmark_request.backends, &benchmark_request.profiles, + &benchmark_request.commit_date, ], ) .await @@ -1563,6 +1568,7 @@ where pr: u32, sha: &str, parent_sha: &str, + commit_date: DateTime, ) -> anyhow::Result<()> { self.conn() .execute( @@ -1571,7 +1577,8 @@ where SET tag = $1, parent_sha = $2, - status = $3 + status = $3, + commit_date = $6 WHERE pr = $4 AND commit_type = 'try' @@ -1583,6 +1590,7 @@ where &BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR, &(pr as i32), &BENCHMARK_REQUEST_STATUS_WAITING_FOR_ARTIFACTS_STR, + &commit_date, ], ) .await @@ -1603,7 +1611,8 @@ where created_at, completed_at, backends, - profiles + profiles, + commit_date FROM benchmark_request WHERE status IN('{BENCHMARK_REQUEST_STATUS_ARTIFACTS_READY_STR}', '{BENCHMARK_REQUEST_STATUS_IN_PROGRESS_STR}')"# ); @@ -1626,6 +1635,7 @@ where let completed_at = row.get::<_, Option>>(6); let backends = row.get::<_, String>(7); let profiles = row.get::<_, String>(8); + let commit_date = row.get::<_, Option>>(9); let pr = pr.map(|v| v as u32); @@ -1640,6 +1650,7 @@ where parent_sha, pr: pr.expect("Try commit in the DB without a PR"), }, + commit_date, created_at, status, backends, @@ -1652,6 +1663,7 @@ where .expect("Master commit in the DB without a parent SHA"), pr: pr.expect("Master commit in the DB without a PR"), }, + commit_date, created_at, status, backends, @@ -1661,6 +1673,7 @@ where commit_type: BenchmarkRequestType::Release { tag: tag.expect("Release commit in the DB without a SHA"), }, + commit_date, created_at, status, backends, @@ -1676,9 +1689,9 @@ where async fn enqueue_benchmark_job( &self, request_tag: &str, - target: &Target, - backend: &CodegenBackend, - profile: &Profile, + target: Target, + backend: CodegenBackend, + profile: Profile, benchmark_set: u32, ) -> anyhow::Result<()> { self.conn() @@ -1736,7 +1749,7 @@ where async fn add_collector_config( &self, collector_name: &str, - target: &Target, + target: Target, benchmark_set: u32, is_active: bool, ) -> anyhow::Result { @@ -1774,7 +1787,7 @@ where let collector_config = CollectorConfig { name: collector_name.into(), - target: *target, + target, benchmark_set: BenchmarkSet(benchmark_set), is_active, last_heartbeat_at: row.get::<_, DateTime>(0), @@ -1783,10 +1796,13 @@ where Ok(collector_config) } - async fn get_collector_config(&self, collector_name: &str) -> anyhow::Result { + async fn get_collector_config( + &self, + collector_name: &str, + ) -> anyhow::Result> { let row = self .conn() - .query_one( + .query_opt( "SELECT target, benchmark_set, @@ -1801,25 +1817,30 @@ where ) .await?; - let collector_config = CollectorConfig { - name: collector_name.into(), - target: Target::from_str(&row.get::<_, String>(0)).map_err(|e| anyhow::anyhow!(e))?, - benchmark_set: BenchmarkSet(row.get::<_, i32>(1) as u32), - is_active: row.get::<_, bool>(2), - last_heartbeat_at: row.get::<_, DateTime>(3), - date_added: row.get::<_, DateTime>(4), - }; - Ok(collector_config) + Ok(row + .map(|row| { + anyhow::Ok(CollectorConfig { + name: collector_name.into(), + target: Target::from_str(row.get::<_, &str>(0)) + .map_err(|e| anyhow::anyhow!(e))?, + benchmark_set: BenchmarkSet(row.get::<_, i32>(1) as u32), + is_active: row.get::<_, bool>(2), + last_heartbeat_at: row.get::<_, DateTime>(3), + date_added: row.get::<_, DateTime>(4), + }) + }) + .transpose()?) } async fn dequeue_benchmark_job( &self, collector_name: &str, - target: &Target, - benchmark_set: &BenchmarkSet, - ) -> anyhow::Result> { + target: Target, + benchmark_set: BenchmarkSet, + ) -> anyhow::Result> { // We take the oldest job from the job_queue matching the benchmark_set, // target and status of 'queued' + // If a job was dequeued, we increment its retry (dequeue) count let row_opt = self .conn() .query_opt( @@ -1837,30 +1858,32 @@ where BY created_at LIMIT 1 FOR UPDATE SKIP LOCKED - ), updated_queue AS ( + ), updated AS ( UPDATE job_queue SET collector_name = $4, started_at = NOW(), - status = $5 + status = $5, + retry = retry + 1 FROM picked WHERE job_queue.id = picked.id - RETURNING - job_queue.id, - job_queue.backend, - job_queue.profile, - job_queue.request_tag, - job_queue.created_at, - job_queue.started_at, - job_queue.retry + RETURNING job_queue.* ) SELECT - * - FROM - updated_queue;", + updated.id, + updated.backend, + updated.profile, + updated.request_tag, + updated.created_at, + updated.started_at, + updated.retry, + br.commit_type, + br.commit_date + FROM updated + JOIN benchmark_request as br ON br.tag = updated.request_tag;", &[ &BENCHMARK_JOB_STATUS_QUEUED_STR, &target, @@ -1876,22 +1899,46 @@ where Some(row) => { let job = BenchmarkJob { id: row.get::<_, i32>(0) as u32, - target: *target, - backend: CodegenBackend::from_str(&row.get::<_, String>(1)) + target, + backend: CodegenBackend::from_str(row.get::<_, &str>(1)) .map_err(|e| anyhow::anyhow!(e))?, - profile: Profile::from_str(&row.get::<_, String>(2)) + profile: Profile::from_str(row.get::<_, &str>(2)) .map_err(|e| anyhow::anyhow!(e))?, request_tag: row.get::<_, String>(3), - benchmark_set: benchmark_set.clone(), + benchmark_set, created_at: row.get::<_, DateTime>(4), // The job is now in an in_progress state status: BenchmarkJobStatus::InProgress { started_at: row.get::<_, DateTime>(5), collector_name: collector_name.into(), }, - retry: row.get::<_, i32>(6) as u32, + deque_counter: row.get::<_, i32>(6) as u32, + }; + let commit_type = row.get::<_, &str>(7); + let commit_date = row.get::<_, Option>>(8); + + let commit_date = Date(commit_date.ok_or_else(|| { + anyhow::anyhow!("Dequeuing job for a benchmark request without commit date") + })?); + let artifact_id = match commit_type { + BENCHMARK_REQUEST_TRY_STR => ArtifactId::Commit(Commit { + sha: job.request_tag.clone(), + date: commit_date, + r#type: CommitType::Try, + }), + BENCHMARK_REQUEST_MASTER_STR => ArtifactId::Commit(Commit { + sha: job.request_tag.clone(), + date: commit_date, + r#type: CommitType::Master, + }), + BENCHMARK_REQUEST_RELEASE_STR => ArtifactId::Tag(job.request_tag.clone()), + _ => panic!( + "Invalid commit type {commit_type} for benchmark request {}", + job.request_tag + ), }; - Ok(Some(job)) + + Ok(Some((job, artifact_id))) } } } @@ -1952,19 +1999,19 @@ where async fn mark_benchmark_job_as_completed( &self, id: u32, - benchmark_job_conclusion: &BenchmarkJobConclusion, + conclusion: BenchmarkJobConclusion, ) -> anyhow::Result<()> { self.conn() .execute( " UPDATE - job_queue + job_queue SET status = $1, completed_at = NOW() WHERE id = $2", - &[&benchmark_job_conclusion.as_str(), &(id as i32)], + &[&conclusion.as_str(), &(id as i32)], ) .await .context("Failed to mark benchmark job as completed")?; @@ -1987,7 +2034,7 @@ fn parse_artifact_id(ty: &str, sha: &str, date: Option>) -> Artifa r#type: CommitType::Try, }), "release" => ArtifactId::Tag(sha.to_owned()), - _ => panic!("unknown artifact type: {:?}", ty), + _ => panic!("unknown artifact type: {ty:?}"), } } diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs index c4391ba07..c5c215035 100644 --- a/database/src/pool/sqlite.rs +++ b/database/src/pool/sqlite.rs @@ -165,13 +165,12 @@ impl Migration { let foreign_col: String = row.get_unwrap(4); panic!( "Foreign key violation encountered during migration\n\ - table: {},\n\ - column: {},\n\ - row_id: {:?},\n\ - foreign table: {},\n\ - foreign column: {}\n\ - migration ID: {}\n", - table, col, row_id, foreign_table, foreign_col, migration_id, + table: {table},\n\ + column: {col},\n\ + row_id: {row_id:?},\n\ + foreign table: {foreign_table},\n\ + foreign column: {foreign_col}\n\ + migration ID: {migration_id}\n", ); }, ) @@ -888,7 +887,7 @@ impl Connection for SqliteConnection { query .query_row(params![&sid, &aid.0], |row| row.get(0)) .unwrap_or_else(|e| { - panic!("{:?}: series={:?}, aid={:?}", e, sid, aid); + panic!("{e:?}: series={sid:?}, aid={aid:?}"); }) }) }) @@ -920,7 +919,7 @@ impl Connection for SqliteConnection { query .query_row(params![&sid, &aid.0], |row| row.get(0)) .unwrap_or_else(|e| { - panic!("{:?}: series={:?}, aid={:?}", e, sid, aid); + panic!("{e:?}: series={sid:?}, aid={aid:?}"); }) }) }) @@ -1289,6 +1288,7 @@ impl Connection for SqliteConnection { _pr: u32, _sha: &str, _parent_sha: &str, + _commit_date: DateTime, ) -> anyhow::Result<()> { no_queue_implementation_abort!() } @@ -1296,9 +1296,9 @@ impl Connection for SqliteConnection { async fn enqueue_benchmark_job( &self, _request_tag: &str, - _target: &Target, - _backend: &CodegenBackend, - _profile: &Profile, + _target: Target, + _backend: CodegenBackend, + _profile: Profile, _benchmark_set: u32, ) -> anyhow::Result<()> { no_queue_implementation_abort!() @@ -1331,23 +1331,26 @@ impl Connection for SqliteConnection { .collect::>()?) } - async fn get_collector_config(&self, _collector_name: &str) -> anyhow::Result { + async fn get_collector_config( + &self, + _collector_name: &str, + ) -> anyhow::Result> { no_queue_implementation_abort!() } async fn dequeue_benchmark_job( &self, _collector_name: &str, - _target: &Target, - _benchmark_set: &BenchmarkSet, - ) -> anyhow::Result> { + _target: Target, + _benchmark_set: BenchmarkSet, + ) -> anyhow::Result> { no_queue_implementation_abort!() } async fn add_collector_config( &self, _collector_name: &str, - _target: &Target, + _target: Target, _benchmark_set: u32, _is_active: bool, ) -> anyhow::Result { @@ -1361,7 +1364,7 @@ impl Connection for SqliteConnection { async fn mark_benchmark_job_as_completed( &self, _id: u32, - _benchmark_job_conculsion: &BenchmarkJobConclusion, + _benchmark_job_conculsion: BenchmarkJobConclusion, ) -> anyhow::Result<()> { no_queue_implementation_abort!() } @@ -1385,6 +1388,6 @@ fn parse_artifact_id(ty: &str, sha: &str, date: Option) -> ArtifactId { r#type: CommitType::Try, }), "release" => ArtifactId::Tag(sha.to_owned()), - _ => panic!("unknown artifact type: {:?}", ty), + _ => panic!("unknown artifact type: {ty:?}"), } } diff --git a/site/src/benchmark_metadata/mod.rs b/site/src/benchmark_metadata/mod.rs index 2eaa94101..d0fa4ebe3 100644 --- a/site/src/benchmark_metadata/mod.rs +++ b/site/src/benchmark_metadata/mod.rs @@ -57,8 +57,7 @@ fn load_compile_benchmark_metadata() -> HashMap c, None => { @@ -108,7 +108,7 @@ pub async fn handle_triage( let summary = match compare_given_commits(start.clone(), end.clone(), metric, ctxt, master_commits) .await - .map_err(|e| format!("error comparing beginning and ending commits: {}", e))? + .map_err(|e| format!("error comparing beginning and ending commits: {e}"))? { Some(summary_comparison) => { let (primary, secondary) = @@ -135,8 +135,8 @@ pub async fn handle_compare( let comparison = compare_given_commits(body.start, end.clone(), body.stat, ctxt, master_commits) .await - .map_err(|e| format!("error comparing commits: {}", e))? - .ok_or_else(|| format!("could not find end commit for bound {:?}", end))?; + .map_err(|e| format!("error comparing commits: {e}"))? + .ok_or_else(|| format!("could not find end commit for bound {end:?}"))?; let conn = ctxt.conn().await; let prev = comparison.prev(master_commits); @@ -505,8 +505,7 @@ async fn write_triage_summary( let mut result = if let Some(pr) = comparison.b.pr { let title = github::pr_title(pr).await; format!( - "{} [#{}](https://github.com/rust-lang/rust/pull/{})", - title, pr, pr + "{title} [#{pr}](https://github.com/rust-lang/rust/pull/{pr})" ) } else { String::from("") @@ -514,7 +513,7 @@ async fn write_triage_summary( let start = &comparison.a.artifact; let end = &comparison.b.artifact; let link = &compare_link(start, end); - write!(&mut result, " [(Comparison Link)]({})\n\n", link).unwrap(); + write!(&mut result, " [(Comparison Link)]({link})\n\n").unwrap(); write_summary_table(primary, secondary, true, &mut result); @@ -725,7 +724,7 @@ async fn compare_given_commits( let idx = ctxt.index.load(); let a = ctxt .artifact_id_for_bound(start.clone(), true) - .ok_or(format!("could not find start commit for bound {:?}", start))?; + .ok_or(format!("could not find start commit for bound {start:?}"))?; let b = match ctxt.artifact_id_for_bound(end.clone(), false) { Some(b) => b, None => return Ok(None), @@ -1449,15 +1448,13 @@ async fn generate_report( .iter() .map(|github::PullRequest { title, number }| { format!( - "- [#{} {}](https://github.com/rust-lang/rust/pull/{})", - number, title, number + "- [#{number} {title}](https://github.com/rust-lang/rust/pull/{number})" ) }) .collect::>() .join("\n"), Err(e) => format!( - "An **error** occurred when finding the untriaged PRs: {}", - e + "An **error** occurred when finding the untriaged PRs: {e}" ), }; let num_regressions = regressions.len(); @@ -1532,8 +1529,7 @@ fn compare_link(start: &ArtifactId, end: &ArtifactId) -> String { ArtifactId::Commit(c) => &c.sha, }; format!( - "https://perf.rust-lang.org/compare.html?start={}&end={}&stat=instructions:u", - start, end + "https://perf.rust-lang.org/compare.html?start={start}&end={end}&stat=instructions:u" ) } @@ -1758,8 +1754,7 @@ mod tests { // making the tables hard to read when printed. if result != expected { panic!( - "output mismatch:\nexpected:\n{}actual:\n{}", - expected, result + "output mismatch:\nexpected:\n{expected}actual:\n{result}" ); } } diff --git a/site/src/github.rs b/site/src/github.rs index d1f2d32b2..83005c9e2 100644 --- a/site/src/github.rs +++ b/site/src/github.rs @@ -4,11 +4,11 @@ pub mod comparison_summary; use crate::api::github::Commit; use crate::job_queue::run_new_queue; use crate::load::{MissingReason, SiteCtxt, TryCommit}; +use chrono::{DateTime, Utc}; +use serde::Deserialize; use std::sync::LazyLock; use std::time::Duration; -use serde::Deserialize; - type BoxedError = Box; pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-lang/rust"; @@ -35,7 +35,7 @@ pub async fn unroll_rollup( let commit_link = |sha: &str| format!("https://github.com/rust-lang/rust/commit/{sha}"); let format_commit = |s: &str, truncate: bool| { - let display = truncate.then(|| s.split_at(10).0).unwrap_or(s); + let display = if truncate { s.split_at(10).0 } else { s }; format!("[{display}]({})", commit_link(s)) }; @@ -236,13 +236,18 @@ pub async fn rollup_pr_number( async fn attach_shas_to_try_benchmark_request( conn: &dyn database::pool::Connection, - pr: u32, - sha: &str, - parent_sha: &str, + pr_number: u32, + commit: &TryCommit, + commit_date: DateTime, ) { if run_new_queue() { if let Err(e) = conn - .attach_shas_to_try_benchmark_request(pr, sha, parent_sha) + .attach_shas_to_try_benchmark_request( + pr_number, + &commit.sha, + &commit.parent_sha, + commit_date, + ) .await { log::error!("Failed to add shas to try commit {}", e); @@ -279,8 +284,8 @@ pub async fn enqueue_shas( attach_shas_to_try_benchmark_request( &*conn, pr_number, - &try_commit.sha, - &try_commit.parent_sha, + &try_commit, + commit_response.commit.committer.date, ) .await; @@ -435,7 +440,7 @@ pub(crate) async fn untriaged_perf_regressions() -> Result, Box /// Get the title of a PR with the given number pub(crate) async fn pr_title(pr: u32) -> String { - let url = format!("https://api.github.com/repos/rust-lang/rust/pulls/{}", pr); + let url = format!("https://api.github.com/repos/rust-lang/rust/pulls/{pr}"); let request = github_request(&url); async fn send(request: reqwest::RequestBuilder) -> Result { @@ -447,7 +452,7 @@ pub(crate) async fn pr_title(pr: u32) -> String { .ok_or_else(malformed_json_error)? .to_owned()) } - let request_dbg = format!("{:?}", request); + let request_dbg = format!("{request:?}"); match send(request).await { Ok(t) => t, Err(e) => { @@ -465,7 +470,7 @@ fn github_request(url: &str) -> reqwest::RequestBuilder { .header("User-Agent", "rustc-perf"); if let Ok(token) = std::env::var("GITHUB_TOKEN") { let mut value = - reqwest::header::HeaderValue::from_str(&format!("token {}", token)).unwrap(); + reqwest::header::HeaderValue::from_str(&format!("token {token}")).unwrap(); value.set_sensitive(true); request = request.header("Authorization", value); } diff --git a/site/src/github/client.rs b/site/src/github/client.rs index 8b463a5f9..4e8a7df4a 100644 --- a/site/src/github/client.rs +++ b/site/src/github/client.rs @@ -226,7 +226,7 @@ impl Client { let resp = self.send(req).await; if let Err(e) = resp { - eprintln!("failed to post comment: {:?}", e); + eprintln!("failed to post comment: {e:?}"); } } @@ -263,7 +263,7 @@ impl GraphQLClient { headers.insert(USER_AGENT, header::HeaderValue::from_static(BOT_USER_AGENT)); headers.insert( header::AUTHORIZATION, - header::HeaderValue::from_str(&format!("token {}", token)).unwrap(), + header::HeaderValue::from_str(&format!("token {token}")).unwrap(), ); let client = reqwest::ClientBuilder::new() diff --git a/site/src/job_queue/mod.rs b/site/src/job_queue/mod.rs index 5131f82d0..cac731f8b 100644 --- a/site/src/job_queue/mod.rs +++ b/site/src/job_queue/mod.rs @@ -68,9 +68,9 @@ async fn create_benchmark_request_releases( .take(20) .collect(); - for (name, date_time) in releases { - if date_time >= cutoff && !index.contains_tag(&name) { - let release_request = BenchmarkRequest::create_release(&name, date_time); + for (name, commit_date) in releases { + if commit_date >= cutoff && !index.contains_tag(&name) { + let release_request = BenchmarkRequest::create_release(&name, commit_date); log::info!("Inserting release benchmark request {release_request:?}"); if let Err(error) = conn.insert_benchmark_request(&release_request).await { log::error!("Failed to insert release benchmark request: {error}"); @@ -198,12 +198,12 @@ pub async fn enqueue_benchmark_request( // Target x benchmark_set x backend x profile -> BenchmarkJob for target in Target::all() { for benchmark_set in 0..benchmark_set_count(target.into()) { - for backend in backends.iter() { - for profile in profiles.iter() { + for &backend in backends.iter() { + for &profile in profiles.iter() { tx.conn() .enqueue_benchmark_job( request_tag, - &target, + target, backend, profile, benchmark_set as u32, @@ -218,7 +218,7 @@ pub async fn enqueue_benchmark_request( tx.conn() .enqueue_benchmark_job( parent_sha, - &target, + target, backend, profile, benchmark_set as u32, @@ -305,43 +305,24 @@ pub async fn cron_main(site_ctxt: Arc>>>, seconds: u #[cfg(test)] mod tests { - use super::*; - use chrono::{Datelike, Duration, TimeZone, Utc}; + use crate::job_queue::build_queue; + use chrono::Utc; + use database::tests::run_postgres_test; use database::{ - tests::run_postgres_test, BenchmarkJobConclusion, BenchmarkSet, CodegenBackend, Profile, + BenchmarkJobConclusion, BenchmarkRequest, BenchmarkRequestStatus, BenchmarkSet, + CodegenBackend, Profile, Target, }; - fn days_ago(day_str: &str) -> chrono::DateTime { - // Walk backwards until the first non-digit, then slice - let days = day_str - .strip_prefix("days") - .unwrap() - .parse::() - .unwrap(); - - let timestamp = Utc::now() - Duration::days(days); - // zero out the seconds - Utc.with_ymd_and_hms( - timestamp.year(), - timestamp.month(), - timestamp.day(), - 0, - 0, - 0, - ) - .unwrap() - } - - fn create_master(sha: &str, parent: &str, pr: u32, age_days: &str) -> BenchmarkRequest { - BenchmarkRequest::create_master(sha, parent, pr, days_ago(age_days)) + fn create_master(sha: &str, parent: &str, pr: u32) -> BenchmarkRequest { + BenchmarkRequest::create_master(sha, parent, pr, Utc::now()) } - fn create_try(pr: u32, age_days: &str) -> BenchmarkRequest { - BenchmarkRequest::create_try_without_artifacts(pr, days_ago(age_days), "", "") + fn create_try(pr: u32) -> BenchmarkRequest { + BenchmarkRequest::create_try_without_artifacts(pr, "", "") } - fn create_release(tag: &str, age_days: &str) -> BenchmarkRequest { - BenchmarkRequest::create_release(tag, days_ago(age_days)) + fn create_release(tag: &str) -> BenchmarkRequest { + BenchmarkRequest::create_release(tag, Utc::now()) } async fn db_insert_requests( @@ -349,7 +330,7 @@ mod tests { requests: &[BenchmarkRequest], ) { for request in requests { - conn.insert_benchmark_request(&request).await.unwrap(); + conn.insert_benchmark_request(request).await.unwrap(); } } @@ -358,21 +339,21 @@ mod tests { request_tag: &str, collector_name: &str, benchmark_set: u32, - target: &Target, + target: Target, ) { /* Create job for the request */ db.enqueue_benchmark_job( request_tag, - &target, - &CodegenBackend::Llvm, - &Profile::Opt, + target, + CodegenBackend::Llvm, + Profile::Opt, benchmark_set, ) .await .unwrap(); - let job = db - .dequeue_benchmark_job(collector_name, &target, &BenchmarkSet(benchmark_set)) + let (job, _) = db + .dequeue_benchmark_job(collector_name, target, BenchmarkSet::new(benchmark_set)) .await .unwrap() .unwrap(); @@ -380,15 +361,14 @@ mod tests { assert_eq!(job.request_tag(), request_tag); /* Mark the job as complete */ - db.mark_benchmark_job_as_completed(job.id(), &BenchmarkJobConclusion::Success) + db.mark_benchmark_job_as_completed(job.id(), BenchmarkJobConclusion::Success) .await .unwrap(); - assert_eq!( + assert!( db.mark_benchmark_request_as_completed(request_tag) .await - .unwrap(), - true + .unwrap() ); } @@ -397,7 +377,7 @@ mod tests { shas: &[&str], collector_name: &str, benchmark_set: u32, - target: &database::Target, + target: database::Target, ) { for sha in shas { complete_request(conn, sha, collector_name, benchmark_set, target).await; @@ -407,7 +387,7 @@ mod tests { fn queue_order_matches(queue: &[BenchmarkRequest], expected: &[&str]) { let queue_shas: Vec<&str> = queue .iter() - .filter_map(|request| request.tag().map(|tag| tag)) + .filter_map(|request| request.tag()) .collect(); assert_eq!(queue_shas, expected) } @@ -416,11 +396,11 @@ mod tests { async fn queue_ordering() { run_postgres_test(|ctx| async { let db = ctx.db_client().connection().await; - let target = &Target::X86_64UnknownLinuxGnu; + let target = Target::X86_64UnknownLinuxGnu; let collector_name = "collector-1"; let benchmark_set = 1; - db.add_collector_config(collector_name, &target, benchmark_set, true) + db.add_collector_config(collector_name, target, benchmark_set, true) .await .unwrap(); /* Key: @@ -482,26 +462,26 @@ mod tests { * +----------------+ **/ let requests = vec![ - create_master("bar", "parent1", 10, "days2"), - create_master("345", "parent2", 11, "days2"), - create_master("aaa", "parent3", 12, "days2"), - create_master("rrr", "parent4", 13, "days2"), - create_master("123", "bar", 14, "days2"), - create_master("foo", "345", 15, "days2"), - create_try(16, "days1"), - create_release("v1.2.3", "days2"), - create_try(17, "days1"), - create_master("mmm", "aaa", 18, "days2"), + create_master("bar", "parent1", 10), + create_master("345", "parent2", 11), + create_master("aaa", "parent3", 12), + create_master("rrr", "parent4", 13), + create_master("123", "bar", 14), + create_master("foo", "345", 15), + create_try(16), + create_release("v1.2.3"), + create_try(17), + create_master("mmm", "aaa", 18), ]; db_insert_requests(&*db, &requests).await; - db.attach_shas_to_try_benchmark_request(16, "try1", "rrr") + db.attach_shas_to_try_benchmark_request(16, "try1", "rrr", Utc::now()) .await .unwrap(); db.update_benchmark_request_status("try1", BenchmarkRequestStatus::InProgress) .await .unwrap(); - db.attach_shas_to_try_benchmark_request(17, "baz", "foo") + db.attach_shas_to_try_benchmark_request(17, "baz", "foo", Utc::now()) .await .unwrap(); @@ -510,7 +490,7 @@ mod tests { &["bar", "345", "aaa", "rrr"], collector_name, benchmark_set, - &target, + target, ) .await; diff --git a/site/src/load.rs b/site/src/load.rs index 6693eff6a..f48afb025 100644 --- a/site/src/load.rs +++ b/site/src/load.rs @@ -745,7 +745,7 @@ mod tests { all_commits, time, ); - assert_eq!(expected, found, "{:#?} != {:#?}", expected, found); + assert_eq!(expected, found, "{expected:#?} != {found:#?}"); } #[test] diff --git a/site/src/main.rs b/site/src/main.rs index 0f507ff6a..afb63c645 100644 --- a/site/src/main.rs +++ b/site/src/main.rs @@ -54,7 +54,7 @@ async fn main() { }) }) .fuse(); - println!("Starting server with port={:?}", port); + println!("Starting server with port={port:?}"); let server = site::server::start(ctxt.clone(), port).fuse(); diff --git a/site/src/request_handlers/bootstrap.rs b/site/src/request_handlers/bootstrap.rs index a1b3faaa9..72af1678f 100644 --- a/site/src/request_handlers/bootstrap.rs +++ b/site/src/request_handlers/bootstrap.rs @@ -38,7 +38,7 @@ pub async fn handle_bootstrap( // We show any line that has at least one point exceeding the // critical line. if v.iter() - .any(|v| v.map_or(false, |v| v.as_secs() >= body.min_seconds as u64)) + .any(|v| v.is_some_and(|v| v.as_secs() >= body.min_seconds as u64)) { Some(( k, diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index 6ccb450fd..17bab3cd1 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -84,8 +84,7 @@ async fn record_try_benchmark_request_without_artifacts( ) { // We only want to run this if the new system is running if run_new_queue() { - let try_request = - BenchmarkRequest::create_try_without_artifacts(pr, chrono::Utc::now(), backends, ""); + let try_request = BenchmarkRequest::create_try_without_artifacts(pr, backends, ""); log::info!("Inserting try benchmark request {try_request:?}"); if let Err(e) = conn.insert_benchmark_request(&try_request).await { log::error!("Failed to insert try benchmark request: {}", e); @@ -309,12 +308,12 @@ pub async fn get_authorized_users() -> Result, String> { .get(&url) .send() .await - .map_err(|err| format!("failed to fetch authorized users: {}", err))? + .map_err(|err| format!("failed to fetch authorized users: {err}"))? .error_for_status() - .map_err(|err| format!("failed to fetch authorized users: {}", err))? + .map_err(|err| format!("failed to fetch authorized users: {err}"))? .json::() .await - .map_err(|err| format!("failed to fetch authorized users: {}", err)) + .map_err(|err| format!("failed to fetch authorized users: {err}")) .map(|perms| perms.github_ids) } diff --git a/site/src/request_handlers/next_artifact.rs b/site/src/request_handlers/next_artifact.rs index 95253d119..2eea831a8 100644 --- a/site/src/request_handlers/next_artifact.rs +++ b/site/src/request_handlers/next_artifact.rs @@ -20,7 +20,7 @@ pub async fn handle_next_artifact(ctxt: Arc) -> next_artifact::Respons let next_commit = ctxt.missing_commits().await.into_iter().next(); let next_commit = if let Some((commit, missing_reason)) = next_commit { - let missing_reason_dbg = format!("{:?}", missing_reason); + let missing_reason_dbg = format!("{missing_reason:?}"); // If we're going to run a master commit next, make sure // it's been enqueued in the pull_request_build table if let MissingReason::Master { diff --git a/site/src/request_handlers/self_profile.rs b/site/src/request_handlers/self_profile.rs index 84c6bbd0f..29383fda9 100644 --- a/site/src/request_handlers/self_profile.rs +++ b/site/src/request_handlers/self_profile.rs @@ -118,7 +118,7 @@ pub async fn handle_self_profile_processed_download( Ok(c) => c, Err(e) => { log::error!("Failed to generate json {:?}", e); - let mut resp = http::Response::new(format!("{:?}", e).into()); + let mut resp = http::Response::new(format!("{e:?}").into()); *resp.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; return resp; } @@ -290,7 +290,7 @@ pub async fn handle_self_profile_raw_download( let resp = match reqwest::get(&url).await { Ok(r) => r, Err(e) => { - let mut resp = http::Response::new(format!("{:?}", e).into()); + let mut resp = http::Response::new(format!("{e:?}").into()); *resp.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; return resp; } @@ -371,7 +371,7 @@ pub async fn handle_self_profile_raw( let scenario = body .scenario .parse::() - .map_err(|e| format!("invalid run name: {:?}", e))?; + .map_err(|e| format!("invalid run name: {e:?}"))?; let conn = ctxt.conn().await; @@ -397,7 +397,7 @@ pub async fn handle_self_profile_raw( if aids_and_cids.iter().any(|(_, v)| *v == cid) { cid } else { - return Err(format!("{} is not a collection ID at this artifact", cid)); + return Err(format!("{cid} is not a collection ID at this artifact")); } } _ => first_cid, @@ -417,9 +417,9 @@ pub async fn handle_self_profile_raw( cid, ); - return match fetch(&cids, cid, format!("{}.mm_profdata.sz", url_prefix)).await { + return match fetch(&cids, cid, format!("{url_prefix}.mm_profdata.sz")).await { Ok(fetched) => Ok(fetched), - Err(new_error) => Err(format!("mm_profdata download failed: {:?}", new_error,)), + Err(new_error) => Err(format!("mm_profdata download failed: {new_error:?}",)), }; async fn fetch( @@ -431,7 +431,7 @@ pub async fn handle_self_profile_raw( .head(&url) .send() .await - .map_err(|e| format!("fetching artifact: {:?}", e))?; + .map_err(|e| format!("fetching artifact: {e:?}"))?; if !resp.status().is_success() { return Err(format!( "Artifact did not resolve successfully: {:?} received", @@ -458,7 +458,7 @@ pub async fn handle_self_profile( let scenario = body .scenario .parse::() - .map_err(|e| format!("invalid run name: {:?}", e))?; + .map_err(|e| format!("invalid run name: {e:?}"))?; let index = ctxt.index.load(); let query = selector::CompileBenchmarkQuery::default() @@ -471,7 +471,7 @@ pub async fn handle_self_profile( let find_aid = |commit: &str| { index .artifact_id_for_commit(commit) - .ok_or(format!("could not find artifact {}", commit)) + .ok_or(format!("could not find artifact {commit}")) }; let mut commits = vec![find_aid(&body.commit)?]; diff --git a/site/src/selector.rs b/site/src/selector.rs index 0d8326656..610c2e084 100644 --- a/site/src/selector.rs +++ b/site/src/selector.rs @@ -84,7 +84,7 @@ impl StatisticSeriesExt for StatisticSeries { ctxt: &SiteCtxt, query: Q, ) -> Result>, String> { - let dumped = format!("{:?}", query); + let dumped = format!("{query:?}"); let index = ctxt.index.load(); let mut conn = ctxt.conn().await; diff --git a/site/src/self_profile.rs b/site/src/self_profile.rs index 1fee911e7..cd7ff6127 100644 --- a/site/src/self_profile.rs +++ b/site/src/self_profile.rs @@ -221,14 +221,14 @@ async fn download_and_analyze_self_profile( let profiling_data = match fetch_raw_self_profile_data(*anum, benchmark, profile, scenario, *cid).await { Ok(d) => extract_profiling_data(d) - .map_err(|e| format!("error extracting self profiling data: {}", e))?, + .map_err(|e| format!("error extracting self profiling data: {e}"))?, Err(e) => return Err(format!("could not fetch raw profile data: {e:?}")), }; let compilation_sections = compute_compilation_sections(&profiling_data); let profiling_data = profiling_data.perform_analysis(); let profile = - get_self_profile_data(metric, &profiling_data).map_err(|e| format!("{}: {}", aid, e))?; + get_self_profile_data(metric, &profiling_data).map_err(|e| format!("{aid}: {e}"))?; Ok(SelfProfileWithAnalysis { profile, profiling_data, diff --git a/site/src/self_profile/crox.rs b/site/src/self_profile/crox.rs index fc803b78d..a32ab0d16 100644 --- a/site/src/self_profile/crox.rs +++ b/site/src/self_profile/crox.rs @@ -120,7 +120,7 @@ fn get_args(full_event: &analyzeme::Event) -> Option> { .additional_data .iter() .enumerate() - .map(|(i, arg)| (format!("arg{}", i), arg.to_string())) + .map(|(i, arg)| (format!("arg{i}"), arg.to_string())) .collect(), ) } else { diff --git a/site/src/self_profile/flamegraph.rs b/site/src/self_profile/flamegraph.rs index f4a19bbd6..d8b982b00 100644 --- a/site/src/self_profile/flamegraph.rs +++ b/site/src/self_profile/flamegraph.rs @@ -11,7 +11,7 @@ pub fn generate(title: &str, self_profile_data: Vec, _: Opt) -> anyhow::Resu let recorded_stacks = collapse_stacks(&profiling_data) .iter() - .map(|(unique_stack, count)| format!("{} {}", unique_stack, count)) + .map(|(unique_stack, count)| format!("{unique_stack} {count}")) .collect::>(); let mut file = Vec::new(); diff --git a/site/src/server.rs b/site/src/server.rs index af4889aa1..1452cafc6 100644 --- a/site/src/server.rs +++ b/site/src/server.rs @@ -269,8 +269,7 @@ impl Server { .status(StatusCode::OK) .header_typed(ContentType::text_utf8()) .body(hyper::Body::from(format!( - "Refreshed too recently ({:?} ago). Please wait.", - elapsed + "Refreshed too recently ({elapsed:?} ago). Please wait." ))) .unwrap(); } @@ -458,7 +457,7 @@ async fn serve_req(server: Server, req: Request) -> Result = server.ctxt.read().as_ref().unwrap().clone(); let mut body = Vec::new(); while let Some(chunk) = body_stream.next().await { - let chunk = chunk.map_err(|e| ServerError(format!("failed to read chunk: {:?}", e)))?; + let chunk = chunk.map_err(|e| ServerError(format!("failed to read chunk: {e:?}")))?; body.extend_from_slice(&chunk); // More than 10 MB of data if body.len() > 1024 * 1024 * 10 { @@ -511,7 +510,7 @@ async fn serve_req(server: Server, req: Request) -> Result Ok(http::Response::builder() .status(StatusCode::OK) - .body(hyper::Body::from(format!("unknown event: {}", event))) + .body(hyper::Body::from(format!("unknown event: {event}"))) .unwrap()), } } @@ -568,8 +567,7 @@ where .header_typed(ContentType::text_utf8()) .status(StatusCode::BAD_REQUEST) .body(hyper::Body::from(format!( - "Failed to deserialize request: {:?}", - err + "Failed to deserialize request: {err:?}" ))) .unwrap()) } @@ -595,8 +593,7 @@ where .header_typed(ContentType::text_utf8()) .status(StatusCode::BAD_REQUEST) .body(hyper::Body::from(format!( - "Failed to deserialize request {}: {:?}", - uri, err, + "Failed to deserialize request {uri}: {err:?}", ))) .unwrap()), } @@ -630,7 +627,7 @@ async fn handle_fs_path( async fn resolve_template(path: &str) -> Vec { TEMPLATES - .get_template(&format!("pages/{}", path)) + .get_template(&format!("pages/{path}")) .await .unwrap() } @@ -803,7 +800,7 @@ async fn run_server(ctxt: Arc>>>, addr: SocketAddr) }); let server = hyper::server::Server::bind(&addr).serve(svc); if let Err(e) = server.await { - eprintln!("server error: {:?}", e); + eprintln!("server error: {e:?}"); } }