Record timings for smaller steps than whole run timing

This should also provide the baseline for progress reporting on the UI (and
eventually -- estimates of completion time!). It should also mean that we can
more easily alert on things like "step taking 2x longer than it should".

Author: Mark-Simulacrum

collector/src/main.rs

@@ -194,8 +194,15 @@ fn bench(
     iterations: usize,
     self_profile: bool,
 ) -> BenchmarkErrors {
-    let status_conn = rt.block_on(pool.connection());
     let mut conn = rt.block_on(pool.connection());
+    let status_conn;
+    let status_conn: Option<&dyn database::Connection> =
+        if conn.separate_transaction_for_collector() {
+            status_conn = rt.block_on(pool.connection());
+            Some(&*status_conn)
+        } else {
+            None
+        };
     let mut errors = BenchmarkErrors::new();
     eprintln!("Benchmarking {} for triple {}", cid, compiler.triple);
 
@@ -229,9 +236,17 @@ fn bench(
         .iter()
         .map(|b| b.name.to_string())
         .collect::<Vec<_>>();
-    rt.block_on(status_conn.collector_start(interned_cid, &steps));
+    rt.block_on(
+        status_conn
+            .unwrap_or_else(|| &*tx.conn())
+            .collector_start(interned_cid, &steps),
+    );
     for (nth_benchmark, benchmark) in benchmarks.iter().enumerate() {
-        rt.block_on(status_conn.collector_start_step(interned_cid, &benchmark.name.to_string()));
+        rt.block_on(
+            status_conn
+                .unwrap_or_else(|| &*tx.conn())
+                .collector_start_step(interned_cid, &benchmark.name.to_string()),
+        );
         rt.block_on(
             tx.conn()
                 .record_benchmark(benchmark.name.0.as_str(), benchmark.supports_stable()),
@@ -262,7 +277,11 @@ fn bench(
                 &format!("{:?}", s),
             ));
         };
-        rt.block_on(status_conn.collector_end_step(interned_cid, &benchmark.name.to_string()));
+        rt.block_on(
+            status_conn
+                .unwrap_or_else(|| &*tx.conn())
+                .collector_end_step(interned_cid, &benchmark.name.to_string()),
+        );
     }
     let end = start.elapsed();
 

database/src/pool.rs

@@ -67,6 +67,18 @@ pub trait Connection: Send + Sync {
     async fn collector_start(&self, aid: ArtifactIdNumber, steps: &[String]);
     async fn collector_start_step(&self, aid: ArtifactIdNumber, step: &str);
     async fn collector_end_step(&self, aid: ArtifactIdNumber, step: &str);
+
+    // This returns `true` if the collector commands can be placed in a separate
+    // transaction.
+    //
+    // Currently, the sqlite backend does not support "regular" usage where they
+    // are used for genuine progress reporting. sqlite does not support
+    // concurrent writers -- it will return an error (or wait, if a busy timeout
+    // is configured).
+    //
+    // For now we don't care much as sqlite is not used in production and in
+    // local usage you can just look at the logs.
+    fn separate_transaction_for_collector(&self) -> bool;
 }
 
 #[async_trait::async_trait]

database/src/pool/postgres.rs

@@ -802,7 +802,7 @@ where
     async fn collector_start_step(&self, aid: ArtifactIdNumber, step: &str) {
         self.conn()
             .execute(
-                "update collector_progress set start = CURRENT_TIMESTAMP \
+                "update collector_progress set start = statement_timestamp() \
                 where aid = $1 and step = $2 and start is null and end is null;",
                 &[&(aid.0 as i16), &step],
             )
@@ -812,11 +812,14 @@ where
     async fn collector_end_step(&self, aid: ArtifactIdNumber, step: &str) {
         self.conn()
             .execute(
-                "update collector_progress set end = CURRENT_TIMESTAMP \
+                "update collector_progress set end = statement_timestamp() \
                 where aid = $1 and step = $2 and start is not null and end is null;",
                 &[&(aid.0 as i16), &step],
             )
             .await
             .unwrap();
     }
+    fn separate_transaction_for_collector(&self) -> bool {
+        true
+    }
 }

database/src/pool/sqlite.rs

@@ -144,8 +144,8 @@ static MIGRATIONS: &[&str] = &[
     create table collector_progress(
         aid integer not null references artifact(id) on delete cascade on update cascade,
         step text not null,
-        start timestamp without time zone,
-        end timestamp without time zone
+        start integer,
+        end integer
     );
     "#,
 ];
@@ -653,7 +653,7 @@ impl Connection for SqliteConnection {
     async fn collector_start_step(&self, aid: ArtifactIdNumber, step: &str) {
         self.raw_ref()
             .execute(
-                "update collector_progress set start = now \
+                "update collector_progress set start = strftime('%s','now') \
             where aid = ? and step = ? and start is null;",
                 params![&aid.0, &step],
             )
@@ -662,10 +662,13 @@ impl Connection for SqliteConnection {
     async fn collector_end_step(&self, aid: ArtifactIdNumber, step: &str) {
         self.raw_ref()
             .execute(
-                "update collector_progress set end = now \
+                "update collector_progress set end = strftime('%s','now') \
             where aid = ? and step = ? and start is not null and end is null;",
                 params![&aid.0, &step],
             )
             .unwrap();
     }
+    fn separate_transaction_for_collector(&self) -> bool {
+        false
+    }
 }