Merge pull request #900 from rylev/artifact_id

Keep usage of aid and cid straight

Author: rylev

collector/src/execute.rs

@@ -557,10 +557,10 @@ pub struct MeasureProcessor<'a> {
     krate: &'a BenchmarkName,
     conn: &'a mut dyn database::Connection,
     artifact: &'a database::ArtifactId,
-    cid: database::ArtifactIdNumber,
+    artifact_row_id: database::ArtifactIdNumber,
     upload: Option<Upload>,
     is_first_collection: bool,
-    self_profile: bool,
+    is_self_profile: bool,
     tries: u8,
 }
 
@@ -570,8 +570,8 @@ impl<'a> MeasureProcessor<'a> {
         conn: &'a mut dyn database::Connection,
         krate: &'a BenchmarkName,
         artifact: &'a database::ArtifactId,
-        cid: database::ArtifactIdNumber,
-        self_profile: bool,
+        artifact_row_id: database::ArtifactIdNumber,
+        is_self_profile: bool,
     ) -> Self {
         // Check we have `perf` or (`xperf.exe` and `tracelog.exe`)  available.
         if cfg!(unix) {
@@ -596,10 +596,9 @@ impl<'a> MeasureProcessor<'a> {
             conn,
             krate,
             artifact,
-            cid,
+            artifact_row_id,
             is_first_collection: true,
-            // Command::new("summarize").status().is_ok()
-            self_profile,
+            is_self_profile,
             tries: 0,
         }
     }
@@ -644,14 +643,14 @@ impl<'a> MeasureProcessor<'a> {
                     u.wait();
                 }
                 let prefix = PathBuf::from("self-profile")
-                    .join(self.cid.0.to_string())
+                    .join(self.artifact_row_id.0.to_string())
                     .join(self.krate.0.as_str())
                     .join(profile.to_string())
                     .join(cache.to_id());
                 self.upload = Some(Upload::new(prefix, collection, files));
                 self.rt.block_on(self.conn.record_raw_self_profile(
                     collection,
-                    self.cid,
+                    self.artifact_row_id,
                     self.krate.0.as_str(),
                     profile,
                     cache,
@@ -663,7 +662,7 @@ impl<'a> MeasureProcessor<'a> {
         for (stat, value) in stats.0.iter() {
             buf.push(self.conn.record_statistic(
                 collection,
-                self.cid,
+                self.artifact_row_id,
                 self.krate.0.as_str(),
                 profile,
                 cache,
@@ -674,12 +673,12 @@ impl<'a> MeasureProcessor<'a> {
 
         if let Some(sp) = &stats.1 {
             let conn = &*self.conn;
-            let cid = self.cid;
+            let artifact_row_id = self.artifact_row_id;
             let krate = self.krate.0.as_str();
             for qd in &sp.query_data {
                 buf.push(conn.record_self_profile_query(
                     collection,
-                    cid,
+                    artifact_row_id,
                     krate,
                     profile,
                     cache,
@@ -788,7 +787,7 @@ impl Upload {
 
 impl<'a> Processor for MeasureProcessor<'a> {
     fn profiler(&self, _build: BuildKind) -> Profiler {
-        if self.is_first_collection && self.self_profile {
+        if self.is_first_collection && self.is_self_profile {
             if cfg!(unix) {
                 Profiler::PerfStatSelfProfile
             } else {
@@ -865,7 +864,13 @@ impl<'a> Processor for MeasureProcessor<'a> {
     }
 
     fn measure_rustc(&mut self, compiler: Compiler<'_>) -> anyhow::Result<()> {
-        rustc::measure(self.rt, self.conn, compiler, self.artifact, self.cid)
+        rustc::measure(
+            self.rt,
+            self.conn,
+            compiler,
+            self.artifact,
+            self.artifact_row_id,
+        )
     }
 }
 

collector/src/main.rs

@@ -186,20 +186,23 @@ impl BenchmarkErrors {
 fn bench(
     rt: &mut Runtime,
     pool: database::Pool,
-    cid: &ArtifactId,
+    artifact_id: &ArtifactId,
     build_kinds: &[BuildKind],
     run_kinds: &[RunKind],
     compiler: Compiler<'_>,
     benchmarks: &[Benchmark],
     iterations: Option<usize>,
-    self_profile: bool,
+    is_self_profile: bool,
 ) -> BenchmarkErrors {
     let mut conn = rt.block_on(pool.connection());
     let mut errors = BenchmarkErrors::new();
-    eprintln!("Benchmarking {} for triple {}", cid, compiler.triple);
+    eprintln!(
+        "Benchmarking {} for triple {}",
+        artifact_id, compiler.triple
+    );
 
     let has_measureme = Command::new("summarize").output().is_ok();
-    if self_profile {
+    if is_self_profile {
         assert!(
             has_measureme,
             "needs `summarize` in PATH for self profile.\n\
@@ -214,20 +217,20 @@ fn bench(
 
     // Make sure there is no observable time when the artifact ID is available
     // but the in-progress steps are not.
-    let interned_cid = {
+    let artifact_row_id = {
         let mut tx = rt.block_on(conn.transaction());
-        let interned_cid = rt.block_on(tx.conn().artifact_id(&cid));
-        rt.block_on(tx.conn().collector_start(interned_cid, &steps));
+        let artifact_row_id = rt.block_on(tx.conn().artifact_id(&artifact_id));
+        rt.block_on(tx.conn().collector_start(artifact_row_id, &steps));
 
         rt.block_on(tx.commit()).unwrap();
-        interned_cid
+        artifact_row_id
     };
 
     let start = Instant::now();
     let mut skipped = false;
     for (nth_benchmark, benchmark) in benchmarks.iter().enumerate() {
         let is_fresh =
-            rt.block_on(conn.collector_start_step(interned_cid, &benchmark.name.to_string()));
+            rt.block_on(conn.collector_start_step(artifact_row_id, &benchmark.name.to_string()));
         if !is_fresh {
             skipped = true;
             eprintln!("skipping {} -- already benchmarked", benchmark.name);
@@ -247,9 +250,9 @@ fn bench(
             rt,
             tx.conn(),
             &benchmark.name,
-            &cid,
-            interned_cid,
-            self_profile,
+            &artifact_id,
+            artifact_row_id,
+            is_self_profile,
         );
         let result =
             benchmark.measure(&mut processor, build_kinds, run_kinds, compiler, iterations);
@@ -260,14 +263,14 @@ fn bench(
             );
             errors.incr();
             rt.block_on(tx.conn().record_error(
-                interned_cid,
+                artifact_row_id,
                 benchmark.name.0.as_str(),
                 &format!("{:?}", s),
             ));
         };
         rt.block_on(
             tx.conn()
-                .collector_end_step(interned_cid, &benchmark.name.to_string()),
+                .collector_end_step(artifact_row_id, &benchmark.name.to_string()),
         );
         rt.block_on(tx.commit()).expect("committed");
     }
@@ -281,7 +284,7 @@ fn bench(
     if skipped {
         log::info!("skipping duration record -- skipped parts of run");
     } else {
-        rt.block_on(conn.record_duration(interned_cid, end));
+        rt.block_on(conn.record_duration(artifact_row_id, end));
     }
 
     rt.block_on(async move {
@@ -574,7 +577,7 @@ fn main_result() -> anyhow::Result<i32> {
             let include = sub_m.value_of("INCLUDE");
             let run_kinds = run_kinds_from_arg(&sub_m.value_of("RUNS"))?;
             let rustdoc = sub_m.value_of("RUSTDOC");
-            let self_profile = sub_m.is_present("SELF_PROFILE");
+            let is_self_profile = sub_m.is_present("SELF_PROFILE");
 
             let pool = database::Pool::open(db);
 
@@ -597,7 +600,7 @@ fn main_result() -> anyhow::Result<i32> {
                 },
                 &benchmarks,
                 Some(1),
-                self_profile,
+                is_self_profile,
             );
             res.fail_if_nonzero()?;
             Ok(0)
@@ -609,7 +612,7 @@ fn main_result() -> anyhow::Result<i32> {
 
             // Options
             let db = sub_m.value_of("DB").unwrap_or(default_db);
-            let self_profile = sub_m.is_present("SELF_PROFILE");
+            let is_self_profile = sub_m.is_present("SELF_PROFILE");
 
             println!("processing commits");
             let client = reqwest::blocking::Client::new();
@@ -646,7 +649,7 @@ fn main_result() -> anyhow::Result<i32> {
                 Compiler::from_sysroot(&sysroot),
                 &benchmarks,
                 next.runs.map(|v| v as usize),
-                self_profile,
+                is_self_profile,
             );
 
             client.post(&format!("{}/perf/onpush", site_url)).send()?;
@@ -722,7 +725,7 @@ fn main_result() -> anyhow::Result<i32> {
                 },
                 &benchmarks,
                 Some(3),
-                /* self_profile */ false,
+                /* is_self_profile */ false,
             );
             res.fail_if_nonzero()?;
             Ok(0)

database/src/lib.rs

@@ -417,13 +417,21 @@ impl From<Commit> for ArtifactId {
 
 #[async_trait::async_trait]
 pub trait SeriesType: Sized {
-    async fn get(conn: &dyn pool::Connection, series: u32, cid: ArtifactIdNumber) -> Option<Self>;
+    async fn get(
+        conn: &dyn pool::Connection,
+        series: u32,
+        artifact_row_id: ArtifactIdNumber,
+    ) -> Option<Self>;
 }
 
 #[async_trait::async_trait]
 impl SeriesType for f64 {
-    async fn get(conn: &dyn pool::Connection, series: u32, cid: ArtifactIdNumber) -> Option<Self> {
-        conn.get_pstats(&[series], &[Some(cid)]).await[0][0]
+    async fn get(
+        conn: &dyn pool::Connection,
+        series: u32,
+        artifact_row_id: ArtifactIdNumber,
+    ) -> Option<Self> {
+        conn.get_pstats(&[series], &[Some(artifact_row_id)]).await[0][0]
     }
 }
 
@@ -440,13 +448,18 @@ pub struct QueryDatum {
 
 #[async_trait::async_trait]
 impl SeriesType for QueryDatum {
-    async fn get(conn: &dyn pool::Connection, series: u32, cid: ArtifactIdNumber) -> Option<Self> {
-        conn.get_self_profile_query(series, cid).await
+    async fn get(
+        conn: &dyn pool::Connection,
+        series: u32,
+        artifact_row_id: ArtifactIdNumber,
+    ) -> Option<Self> {
+        conn.get_self_profile_query(series, artifact_row_id).await
     }
 }
 #[derive(Hash, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub struct LabelId(pub u8, pub u32);
 
+/// A database row ID for an artifact in the artifact table
 #[derive(Serialize, Deserialize, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 pub struct ArtifactIdNumber(pub u32);
 
@@ -638,20 +651,24 @@ impl Index {
         conn.load_index().await
     }
 
-    pub fn lookup(&self, path: &DbLabel, cid: &ArtifactId) -> Option<(u32, ArtifactIdNumber)> {
-        let cid = cid.lookup(self)?;
+    pub fn lookup(
+        &self,
+        path: &DbLabel,
+        artifact_id: &ArtifactId,
+    ) -> Option<(u32, ArtifactIdNumber)> {
+        let artifact_row_id = artifact_id.lookup(self)?;
         let series = path.lookup(self)?;
-        Some((series, cid))
+        Some((series, artifact_row_id))
     }
 
     pub async fn get<T: SeriesType>(
         &self,
         db: &mut dyn pool::Connection,
         path: &DbLabel,
-        cid: &ArtifactId,
+        artifact_id: &ArtifactId,
     ) -> Option<T> {
-        let (series, cid) = self.lookup(path, cid)?;
-        T::get(db, series, cid).await
+        let (series, artifact_row_id) = self.lookup(path, artifact_id)?;
+        T::get(db, series, artifact_row_id).await
     }
 
     pub fn artifacts(&self) -> impl Iterator<Item = &'_ str> + '_ {

database/src/pool.rs

@@ -75,21 +75,22 @@ pub trait Connection: Send + Sync {
     async fn get_pstats(
         &self,
         series: &[u32],
-        cid: &[Option<ArtifactIdNumber>],
+        artifact_row_id: &[Option<ArtifactIdNumber>],
     ) -> Vec<Vec<Option<f64>>>;
     async fn get_self_profile(
         &self,
-        cid: ArtifactIdNumber,
+        artifact_row_id: ArtifactIdNumber,
         crate_: &str,
         profile: &str,
         cache: &str,
     ) -> HashMap<crate::QueryLabel, QueryDatum>;
     async fn get_self_profile_query(
         &self,
         series: u32,
-        cid: ArtifactIdNumber,
+        artifact_row_id: ArtifactIdNumber,
     ) -> Option<QueryDatum>;
-    async fn get_error(&self, cid: ArtifactIdNumber) -> HashMap<String, Option<String>>;
+    async fn get_error(&self, artifact_row_id: ArtifactIdNumber)
+        -> HashMap<String, Option<String>>;
 
     async fn queue_pr(
         &self,