Rollup merge of #153689 - nnethercote:rm-QueryLatchInfo, r=petrochenkov

Eliminate `QueryLatchInfo`.

The boolean `complete` flag indicates whether the `waiters` vec is still in use, which means the `waiters` vec must be empty when `complete` is true. This is achieved by using `drain` on the waiters just after `complete` is set.

We can do better by using the type system to make invalid combinations impossible. This commit removes `complete` and puts the waiters inside an `Option`. `Some` means the query job is still active, and `None` once it is complete. And `QueryLatchInfo` is eliminated.

(The `Arc<Mutex<Option<Vec<Arc<QueryWaiter<'tcx>>>>>>` type is a mouthful, but when it's all in one place I find it easier to understand than when it's split across two types. And we can use `Option` methods like `as_ref`, `as_mut`, and `take`, which is nice.)

r? @petrochenkov

Author: JonathanBrouwer

compiler/rustc_middle/src/query/job.rs

@@ -77,22 +77,15 @@ pub struct QueryWaiter<'tcx> {
     pub cycle: Mutex<Option<CycleError<QueryStackDeferred<'tcx>>>>,
 }
 
-#[derive(Debug)]
-pub struct QueryLatchInfo<'tcx> {
-    pub complete: bool,
-    pub waiters: Vec<Arc<QueryWaiter<'tcx>>>,
-}
-
 #[derive(Clone, Debug)]
 pub struct QueryLatch<'tcx> {
-    pub info: Arc<Mutex<QueryLatchInfo<'tcx>>>,
+    /// The `Option` is `Some(..)` when the job is active, and `None` once completed.
+    pub waiters: Arc<Mutex<Option<Vec<Arc<QueryWaiter<'tcx>>>>>>,
 }
 
 impl<'tcx> QueryLatch<'tcx> {
     fn new() -> Self {
-        QueryLatch {
-            info: Arc::new(Mutex::new(QueryLatchInfo { complete: false, waiters: Vec::new() })),
-        }
+        QueryLatch { waiters: Arc::new(Mutex::new(Some(Vec::new()))) }
     }
 
     /// Awaits for the query job to complete.
@@ -102,10 +95,10 @@ impl<'tcx> QueryLatch<'tcx> {
         query: Option<QueryJobId>,
         span: Span,
     ) -> Result<(), CycleError<QueryStackDeferred<'tcx>>> {
-        let mut info = self.info.lock();
-        if info.complete {
-            return Ok(());
-        }
+        let mut waiters_guard = self.waiters.lock();
+        let Some(waiters) = &mut *waiters_guard else {
+            return Ok(()); // already complete
+        };
 
         let waiter =
             Arc::new(QueryWaiter { query, span, cycle: Mutex::new(None), condvar: Condvar::new() });
@@ -114,17 +107,17 @@ impl<'tcx> QueryLatch<'tcx> {
         // the `wait` call below, by 1) the `set` method or 2) by deadlock detection.
         // Both of these will remove it from the `waiters` list before resuming
         // this thread.
-        info.waiters.push(Arc::clone(&waiter));
+        waiters.push(Arc::clone(&waiter));
 
         // Awaits the caller on this latch by blocking the current thread.
         // If this detects a deadlock and the deadlock handler wants to resume this thread
         // we have to be in the `wait` call. This is ensured by the deadlock handler
         // getting the self.info lock.
         rustc_thread_pool::mark_blocked();
         tcx.jobserver_proxy.release_thread();
-        waiter.condvar.wait(&mut info);
+        waiter.condvar.wait(&mut waiters_guard);
         // Release the lock before we potentially block in `acquire_thread`
-        drop(info);
+        drop(waiters_guard);
         tcx.jobserver_proxy.acquire_thread();
 
         // FIXME: Get rid of this lock. We have ownership of the QueryWaiter
@@ -139,11 +132,10 @@ impl<'tcx> QueryLatch<'tcx> {
 
     /// Sets the latch and resumes all waiters on it
     fn set(&self) {
-        let mut info = self.info.lock();
-        debug_assert!(!info.complete);
-        info.complete = true;
+        let mut waiters_guard = self.waiters.lock();
+        let waiters = waiters_guard.take().unwrap(); // mark the latch as complete
         let registry = rustc_thread_pool::Registry::current();
-        for waiter in info.waiters.drain(..) {
+        for waiter in waiters {
             rustc_thread_pool::mark_unblocked(&registry);
             waiter.condvar.notify_one();
         }
@@ -152,9 +144,9 @@ impl<'tcx> QueryLatch<'tcx> {
     /// Removes a single waiter from the list of waiters.
     /// This is used to break query cycles.
     pub fn extract_waiter(&self, waiter: usize) -> Arc<QueryWaiter<'tcx>> {
-        let mut info = self.info.lock();
-        debug_assert!(!info.complete);
+        let mut waiters_guard = self.waiters.lock();
+        let waiters = waiters_guard.as_mut().expect("non-empty waiters vec");
         // Remove the waiter from the list of waiters
-        info.waiters.remove(waiter)
+        waiters.remove(waiter)
     }
 }

compiler/rustc_query_impl/src/job.rs

@@ -136,7 +136,7 @@ fn visit_waiters<'tcx>(
 
     // Visit the explicit waiters which use condvars and are resumable
     if let Some(latch) = job_map.latch_of(query) {
-        for (i, waiter) in latch.info.lock().waiters.iter().enumerate() {
+        for (i, waiter) in latch.waiters.lock().as_ref().unwrap().iter().enumerate() {
             if let Some(waiter_query) = waiter.query {
                 // Return a value which indicates that this waiter can be resumed
                 visit(waiter.span, waiter_query).map_break(|_| Some((query, i)))?;