feedback - renaming and enum creation

Author: Jamesbarford

collector/src/bin/collector.rs

@@ -35,7 +35,7 @@ use collector::artifact_stats::{
 use collector::codegen::{codegen_diff, CodegenType};
 use collector::compile::benchmark::category::Category;
 use collector::compile::benchmark::codegen_backend::CodegenBackend;
-use collector::compile::benchmark::compiler_target::CompilerTarget;
+use collector::compile::benchmark::target::Target;
 use collector::compile::benchmark::profile::Profile;
 use collector::compile::benchmark::scenario::Scenario;
 use collector::compile::benchmark::{
@@ -100,7 +100,7 @@ struct CompileBenchmarkConfig {
     iterations: Option<usize>,
     is_self_profile: bool,
     bench_rustc: bool,
-    compiler_targets: Vec<CompilerTarget>,
+    targets: Vec<Target>,
 }
 
 struct RuntimeBenchmarkConfig {
@@ -202,7 +202,7 @@ fn profile_compile(
     scenarios: &[Scenario],
     backends: &[CodegenBackend],
     errors: &mut BenchmarkErrors,
-    compiler_targets: &[CompilerTarget]
+    targets: &[Target]
 ) {
     eprintln!("Profiling {} with {:?}", toolchain.id, profiler);
     if let Profiler::SelfProfile = profiler {
@@ -223,7 +223,7 @@ fn profile_compile(
                 backends,
                 toolchain,
                 Some(1),
-                compiler_targets,
+                targets,
             ));
             eprintln!("Finished benchmark {benchmark_id}");
 
@@ -914,7 +914,7 @@ fn main_result() -> anyhow::Result<i32> {
                 iterations: Some(iterations),
                 is_self_profile: self_profile.self_profile,
                 bench_rustc: bench_rustc.bench_rustc,
-                compiler_targets: vec![CompilerTarget::default()],
+                targets: vec![Target::default()],
             };
 
             run_benchmarks(&mut rt, conn, shared, Some(config), None)?;
@@ -1029,7 +1029,7 @@ fn main_result() -> anyhow::Result<i32> {
                             iterations: runs.map(|v| v as usize),
                             is_self_profile: self_profile.self_profile,
                             bench_rustc: bench_rustc.bench_rustc,
-                            compiler_targets: vec![CompilerTarget::default()],
+                            targets: vec![Target::default()],
                         };
                         let runtime_suite = rt.block_on(load_runtime_benchmarks(
                             conn.as_mut(),
@@ -1142,7 +1142,7 @@ fn main_result() -> anyhow::Result<i32> {
                         scenarios,
                         backends,
                         &mut errors,
-                        &[CompilerTarget::default()],
+                        &[Target::default()],
                     );
                     Ok(id)
                 };
@@ -1741,7 +1741,7 @@ fn bench_published_artifact(
             iterations: Some(3),
             is_self_profile: false,
             bench_rustc: false,
-            compiler_targets: vec![CompilerTarget::default()],
+            targets: vec![Target::default()],
         }),
         Some(RuntimeBenchmarkConfig::new(
             runtime_suite,
@@ -1842,7 +1842,7 @@ fn bench_compile(
                     &config.backends,
                     &shared.toolchain,
                     config.iterations,
-                    &config.compiler_targets,
+                    &config.targets,
                 )))
                 .with_context(|| anyhow::anyhow!("Cannot compile {}", benchmark.name))
             },

collector/src/compile/benchmark/compiler_target.rs

@@ -3,7 +3,7 @@ use crate::compile::benchmark::codegen_backend::CodegenBackend;
 use crate::compile::benchmark::patch::Patch;
 use crate::compile::benchmark::profile::Profile;
 use crate::compile::benchmark::scenario::Scenario;
-use crate::compile::benchmark::compiler_target::CompilerTarget;
+use crate::compile::benchmark::target::Target;
 use crate::compile::execute::{CargoProcess, Processor};
 use crate::toolchain::Toolchain;
 use crate::utils::wait_for_future;
@@ -21,7 +21,7 @@ pub mod codegen_backend;
 pub(crate) mod patch;
 pub mod profile;
 pub mod scenario;
-pub mod compiler_target;
+pub mod target;
 
 fn default_runs() -> usize {
     3
@@ -182,7 +182,7 @@ impl Benchmark {
         cwd: &'a Path,
         profile: Profile,
         backend: CodegenBackend,
-        compiler_target: &'a CompilerTarget,
+        target: Target,
     ) -> CargoProcess<'a> {
         let mut cargo_args = self
             .config
@@ -223,7 +223,7 @@ impl Benchmark {
                 .collect(),
             touch_file: self.config.touch_file.clone(),
             jobserver: None,
-            compiler_target,
+            target,
         }
     }
 
@@ -236,7 +236,7 @@ impl Benchmark {
         backends: &[CodegenBackend],
         toolchain: &Toolchain,
         iterations: Option<usize>,
-        compiler_targets: &[CompilerTarget]
+        targets: &[Target]
     ) -> anyhow::Result<()> {
         if self.config.disabled {
             eprintln!("Skipping {}: disabled", self.name);
@@ -268,11 +268,11 @@ impl Benchmark {
         }
 
         eprintln!("Preparing {}", self.name);
-        let mut target_dirs: Vec<((CodegenBackend, Profile, &CompilerTarget), TempDir)> = vec![];
+        let mut target_dirs: Vec<((CodegenBackend, Profile, Target), TempDir)> = vec![];
         for backend in backends {
             for profile in &profiles {
-                for compiler_target in compiler_targets {
-                    target_dirs.push(((*backend, *profile, &compiler_target), self.make_temp_dir(&self.path)?));
+                for target in targets {
+                    target_dirs.push(((*backend, *profile, *target), self.make_temp_dir(&self.path)?));
                 }
             }
         }
@@ -311,12 +311,12 @@ impl Benchmark {
             )
             .context("jobserver::new")?;
             let mut threads = Vec::with_capacity(target_dirs.len());
-            for ((backend, profile, compiler_target), prep_dir) in &target_dirs {
+            for ((backend, profile, target), prep_dir) in &target_dirs {
                 let server = server.clone();
                 let thread = s.spawn::<_, anyhow::Result<()>>(move || {
                     wait_for_future(async move {
                         let server = server.clone();
-                        self.mk_cargo_process(toolchain, prep_dir.path(), *profile, *backend, compiler_target)
+                        self.mk_cargo_process(toolchain, prep_dir.path(), *profile, *backend, *target)
                             .jobserver(server)
                             .run_rustc(false)
                             .await?;
@@ -350,13 +350,13 @@ impl Benchmark {
         let mut timing_dirs: Vec<ManuallyDrop<TempDir>> = vec![];
 
         let benchmark_start = std::time::Instant::now();
-        for ((backend, profile, compiler_target), prep_dir) in &target_dirs {
+        for ((backend, profile, target), prep_dir) in &target_dirs {
             let backend = *backend;
             let profile = *profile;
-            let compiler_target = *compiler_target;
+            let target = *target;
             eprintln!(
                 "Running {}: {:?} + {:?} + {:?} + {:?}",
-                self.name, profile, scenarios, backend, compiler_target,
+                self.name, profile, scenarios, backend, target,
             );
 
             // We want at least two runs for all benchmarks (since we run
@@ -378,7 +378,7 @@ impl Benchmark {
 
                 // A full non-incremental build.
                 if scenarios.contains(&Scenario::Full) {
-                    self.mk_cargo_process(toolchain, cwd, profile, backend, &compiler_target)
+                    self.mk_cargo_process(toolchain, cwd, profile, backend, target)
                         .processor(processor, Scenario::Full, "Full", None)
                         .run_rustc(true)
                         .await?;
@@ -389,7 +389,7 @@ impl Benchmark {
                     // An incremental  from scratch (slowest incremental case).
                     // This is required for any subsequent incremental builds.
                     if scenarios.iter().any(|s| s.is_incr()) {
-                        self.mk_cargo_process(toolchain, cwd, profile, backend, &compiler_target)
+                        self.mk_cargo_process(toolchain, cwd, profile, backend, target)
                             .incremental(true)
                             .processor(processor, Scenario::IncrFull, "IncrFull", None)
                             .run_rustc(true)
@@ -398,7 +398,7 @@ impl Benchmark {
 
                     // An incremental build with no changes (fastest incremental case).
                     if scenarios.contains(&Scenario::IncrUnchanged) {
-                        self.mk_cargo_process(toolchain, cwd, profile, backend, &compiler_target)
+                        self.mk_cargo_process(toolchain, cwd, profile, backend, target)
                             .incremental(true)
                             .processor(processor, Scenario::IncrUnchanged, "IncrUnchanged", None)
                             .run_rustc(true)
@@ -413,7 +413,7 @@ impl Benchmark {
                             // An incremental build with some changes (realistic
                             // incremental case).
                             let scenario_str = format!("IncrPatched{}", i);
-                            self.mk_cargo_process(toolchain, cwd, profile, backend, &compiler_target)
+                            self.mk_cargo_process(toolchain, cwd, profile, backend, target)
                                 .incremental(true)
                                 .processor(
                                     processor,

collector/src/compile/benchmark/mod.rs

@@ -0,0 +1,10 @@
+#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, serde::Deserialize)]
+pub enum Target {
+    X86_64UnknownLinuxGnu
+}
+
+ impl Default for Target {
+    fn default() -> Self {
+        Self::X86_64UnknownLinuxGnu
+   }
+}

collector/src/compile/benchmark/target.rs

@@ -1,7 +1,7 @@
 use crate::compile::benchmark::codegen_backend::CodegenBackend;
 use crate::compile::benchmark::profile::Profile;
 use crate::compile::benchmark::scenario::Scenario;
-use crate::compile::benchmark::compiler_target::CompilerTarget;
+use crate::compile::benchmark::target::Target;
 use crate::compile::benchmark::BenchmarkName;
 use crate::compile::execute;
 use crate::compile::execute::{
@@ -92,14 +92,18 @@ impl<'a> BenchProcessor<'a> {
         scenario: database::Scenario,
         profile: database::Profile,
         backend: CodegenBackend,
-        compiler_target: &CompilerTarget,
+        target: Target,
         stats: Stats,
     ) {
         let backend = match backend {
             CodegenBackend::Llvm => database::CodegenBackend::Llvm,
             CodegenBackend::Cranelift => database::CodegenBackend::Cranelift,
         };
 
+        let target = match target {
+            Target::X86_64UnknownLinuxGnu => database::Target::X86_64UnknownLinuxGnu
+        };
+
         let mut buf = FuturesUnordered::new();
         for (stat, value) in stats.iter() {
             buf.push(self.conn.record_statistic(
@@ -111,7 +115,7 @@ impl<'a> BenchProcessor<'a> {
                 backend,
                 stat,
                 value,
-                &compiler_target.0,
+                target,
             ));
         }
 
@@ -202,7 +206,7 @@ impl Processor for BenchProcessor<'_> {
                         res.0.stats.retain(|key, _| key.starts_with("size:"));
                     }
 
-                    self.insert_stats(collection, scenario, profile, data.backend, data.compiler_target, res.0)
+                    self.insert_stats(collection, scenario, profile, data.backend, data.target, res.0)
                         .await;
 
                     Ok(Retry::No)

collector/src/compile/execute/bencher.rs

@@ -4,7 +4,7 @@ use crate::compile::benchmark::codegen_backend::CodegenBackend;
 use crate::compile::benchmark::patch::Patch;
 use crate::compile::benchmark::profile::Profile;
 use crate::compile::benchmark::scenario::Scenario;
-use crate::compile::benchmark::compiler_target::CompilerTarget;
+use crate::compile::benchmark::target::Target;
 use crate::compile::benchmark::BenchmarkName;
 use crate::toolchain::Toolchain;
 use crate::utils::fs::EnsureImmutableFile;
@@ -131,7 +131,7 @@ pub struct CargoProcess<'a> {
     pub rustc_args: Vec<String>,
     pub touch_file: Option<String>,
     pub jobserver: Option<jobserver::Client>,
-    pub compiler_target: &'a CompilerTarget,
+    pub target: Target,
 }
 /// Returns an optional list of Performance CPU cores, if the system has P and E cores.
 /// This list *should* be in a format suitable for the `taskset` command.
@@ -276,13 +276,13 @@ impl<'a> CargoProcess<'a> {
     // really.
     pub async fn run_rustc(&mut self, needs_final: bool) -> anyhow::Result<()> {
         log::info!(
-            "run_rustc with incremental={}, profile={:?}, scenario={:?}, patch={:?}, backend={:?}, compiler_target={:?}, phase={}",
+            "run_rustc with incremental={}, profile={:?}, scenario={:?}, patch={:?}, backend={:?}, target={:?}, phase={}",
             self.incremental,
             self.profile,
             self.processor_etc.as_ref().map(|v| v.1),
             self.processor_etc.as_ref().and_then(|v| v.3),
             self.backend,
-            self.compiler_target,
+            self.target,
             if needs_final { "benchmark" } else { "dependencies" }
         );
 
@@ -424,7 +424,7 @@ impl<'a> CargoProcess<'a> {
                     scenario_str,
                     patch,
                     backend: self.backend,
-                    compiler_target: self.compiler_target,
+                    target: self.target,
                 };
                 match processor.process_output(&data, output).await {
                     Ok(Retry::No) => return Ok(()),
@@ -489,7 +489,7 @@ pub struct ProcessOutputData<'a> {
     scenario_str: &'a str,
     patch: Option<&'a Patch>,
     backend: CodegenBackend,
-    compiler_target: &'a CompilerTarget,
+    target: Target,
 }
 
 /// Trait used by `Benchmark::measure()` to provide different kinds of

collector/src/compile/execute/mod.rs

@@ -152,7 +152,7 @@ many times in the `pstat` table.
 
 ```
 sqlite> select * from pstat_series limit 1;
-id          crate       profile     scenario    backend  metric        compiler_target
+id          crate       profile     scenario    backend  metric        target
 ----------  ----------  ----------  ----------  -------  ------------  ------------
 1           helloworld  check       full        llvm     task-clock:u  x86_64-linux-unknown-gnu
 ```

database/schema.md

@@ -45,7 +45,7 @@ async fn main() {
         let sqlite_aid = sqlite_conn.artifact_id(&aid).await;
         let postgres_aid = postgres_conn.artifact_id(&aid).await;
 
-        for (&(benchmark, profile, scenario, backend, metric, ref compiler_target), id) in
+        for (&(benchmark, profile, scenario, backend, metric, target), id) in
             sqlite_idx.compile_statistic_descriptions()
         {
             if benchmarks.insert(benchmark) {
@@ -76,7 +76,7 @@ async fn main() {
                         backend,
                         metric.as_str(),
                         stat,
-                        &compiler_target,
+                        target,
                     )
                     .await;
             }