Merge pull request #1654 from Kobzol/queue-estimation-comment

Add perf. run duration estimation to PR comment

Author: Kobzol

database/src/lib.rs

@@ -668,3 +668,9 @@ pub struct CompileBenchmark {
     pub name: String,
     pub category: String,
 }
+
+#[derive(Debug)]
+pub struct ArtifactCollection {
+    pub duration: Duration,
+    pub end_time: DateTime<Utc>,
+}

database/src/pool.rs

@@ -1,4 +1,4 @@
-use crate::{ArtifactId, ArtifactIdNumber, CompileBenchmark};
+use crate::{ArtifactCollection, ArtifactId, ArtifactIdNumber, CompileBenchmark};
 use crate::{CollectionId, Index, Profile, QueuedCommit, Scenario, Step};
 use chrono::{DateTime, Utc};
 use hashbrown::HashMap;
@@ -153,7 +153,7 @@ pub trait Connection: Send + Sync {
 
     async fn in_progress_steps(&self, aid: &ArtifactId) -> Vec<Step>;
 
-    async fn last_end_time(&self) -> Option<DateTime<Utc>>;
+    async fn last_artifact_collection(&self) -> Option<ArtifactCollection>;
 
     /// Returns the sha of the parent commit, if available.
     ///

database/src/pool/postgres.rs

@@ -1,7 +1,7 @@
 use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction};
 use crate::{
-    ArtifactId, ArtifactIdNumber, Benchmark, CollectionId, Commit, CommitType, CompileBenchmark,
-    Date, Index, Profile, QueuedCommit, Scenario,
+    ArtifactCollection, ArtifactId, ArtifactIdNumber, Benchmark, CollectionId, Commit, CommitType,
+    CompileBenchmark, Date, Index, Profile, QueuedCommit, Scenario,
 };
 use anyhow::Context as _;
 use chrono::{DateTime, TimeZone, Utc};
@@ -1194,18 +1194,21 @@ where
             })
             .collect()
     }
-    async fn last_end_time(&self) -> Option<DateTime<Utc>> {
+    async fn last_artifact_collection(&self) -> Option<ArtifactCollection> {
         self.conn()
             .query_opt(
-                "select date_recorded \
+                "select date_recorded, duration \
                 from artifact_collection_duration \
                 order by date_recorded desc \
                 limit 1;",
                 &[],
             )
             .await
             .unwrap()
-            .map(|r| r.get(0))
+            .map(|r| ArtifactCollection {
+                end_time: r.get(0),
+                duration: Duration::from_secs(r.get::<_, i32>(1) as u64),
+            })
     }
     async fn parent_of(&self, sha: &str) -> Option<String> {
         self.conn()

database/src/pool/sqlite.rs

@@ -1,6 +1,7 @@
 use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction};
 use crate::{
-    ArtifactId, Benchmark, CollectionId, Commit, CommitType, CompileBenchmark, Date, Profile,
+    ArtifactCollection, ArtifactId, Benchmark, CollectionId, Commit, CommitType, CompileBenchmark,
+    Date, Profile,
 };
 use crate::{ArtifactIdNumber, Index, QueryDatum, QueuedCommit};
 use chrono::{DateTime, NaiveDateTime, TimeZone, Utc};
@@ -1149,18 +1150,24 @@ impl Connection for SqliteConnection {
             .collect()
     }
 
-    async fn last_end_time(&self) -> Option<DateTime<Utc>> {
+    async fn last_artifact_collection(&self) -> Option<ArtifactCollection> {
         self.raw_ref()
             .query_row(
-                "select date_recorded + duration \
+                "select date_recorded, duration \
                 from artifact_collection_duration \
                 order by date_recorded desc \
                 limit 1;",
                 params![],
-                |r| Ok(Utc.timestamp_opt(r.get(0)?, 0).unwrap()),
+                |r| {
+                    Ok((
+                        Utc.timestamp_opt(r.get(0)?, 0).unwrap(),
+                        Duration::from_secs(r.get(1)?),
+                    ))
+                },
             )
             .optional()
             .unwrap()
+            .map(|(end_time, duration)| ArtifactCollection { end_time, duration })
     }
 
     async fn parent_of(&self, sha: &str) -> Option<String> {

site/src/github.rs

@@ -3,6 +3,7 @@ pub mod comparison_summary;
 
 use crate::api::github::Commit;
 use crate::load::{SiteCtxt, TryCommit};
+use std::time::Duration;
 
 use serde::Deserialize;
 
@@ -257,22 +258,44 @@ pub async fn enqueue_shas(
             sha: commit_response.sha,
             parent_sha: commit_response.parents.remove(0).sha,
         };
-        let queued = {
-            let conn = ctxt.conn().await;
-            conn.pr_attach_commit(
+        let conn = ctxt.conn().await;
+        let queued = conn
+            .pr_attach_commit(
                 pr_number,
                 &try_commit.sha,
                 &try_commit.parent_sha,
                 Some(commit_response.commit.committer.date),
             )
-            .await
-        };
+            .await;
         if queued {
             if !msg.is_empty() {
                 msg.push('\n');
             }
+
+            let artifacts_in_queue = ctxt.missing_commits().await.len();
+            let last_duration = conn
+                .last_artifact_collection()
+                .await
+                .map(|collection| collection.duration)
+                .unwrap_or(Duration::ZERO);
+
+            // "Guess" that the duration will take about an hour if we don't have data or it's
+            // suspiciously fast.
+            let last_duration = last_duration.max(Duration::from_secs(3600));
+
+            let expected_duration = (last_duration.as_secs() * artifacts_in_queue as u64) as f64;
+
+            // At this point, the queue should also contain the commit that we're mentioning below.
+            let other_artifact_count = artifacts_in_queue.saturating_sub(1);
+            let suffix = if other_artifact_count == 1 { "" } else { "s" };
+            let queue_msg = format!(
+                r#"There are currently {other_artifact_count} other artifact{suffix} in the [queue](https://perf.rust-lang.org/status.html).
+It will probably take at least ~{:.2} hours until the benchmark run finishes."#,
+                (expected_duration / 3600.0)
+            );
+
             msg.push_str(&format!(
-                "Queued {} with parent {}, future [comparison URL]({}).",
+                "Queued {} with parent {}, future [comparison URL]({}).\n{queue_msg}",
                 try_commit.sha,
                 try_commit.parent_sha,
                 try_commit.comparison_url(),

site/src/request_handlers/status_page.rs

@@ -34,9 +34,7 @@ pub async fn handle_status_page(ctxt: Arc<SiteCtxt>) -> status::Response {
     };
 
     let errors = if let Some(last) = &last_commit {
-        ctxt.conn()
-            .await
-            .get_error(ArtifactId::from(last.clone()).lookup(&idx).unwrap())
+        conn.get_error(ArtifactId::from(last.clone()).lookup(&idx).unwrap())
             .await
     } else {
         Default::default()
@@ -54,7 +52,10 @@ pub async fn handle_status_page(ctxt: Arc<SiteCtxt>) -> status::Response {
         benchmarks: benchmark_state,
         missing,
         current,
-        most_recent_end: conn.last_end_time().await.map(|d| d.timestamp()),
+        most_recent_end: conn
+            .last_artifact_collection()
+            .await
+            .map(|d| d.end_time.timestamp()),
     }
 }