Unrolled build for #152229

Rollup merge of #152229 - Zoxc:rem-det-pick, r=nnethercote

Remove deterministic picking from query cycle handling

This removes the deterministic picking of queries from the query cycle handling. The sets we pick from are themselves non-deterministic, making this ineffective. There may be additional entry points to the cycle which we won't discover until after we resume from the cycle handler and run more code.

Author: rust-timer

Cargo.lock

@@ -4492,7 +4492,6 @@ dependencies = [
  "rustc_abi",
  "rustc_data_structures",
  "rustc_errors",
- "rustc_hashes",
  "rustc_hir",
  "rustc_index",
  "rustc_macros",

compiler/rustc_middle/src/query/stack.rs

@@ -4,7 +4,6 @@ use std::mem::transmute;
 use std::sync::Arc;
 
 use rustc_data_structures::sync::{DynSend, DynSync};
-use rustc_hashes::Hash64;
 use rustc_hir::def::DefKind;
 use rustc_span::Span;
 use rustc_span::def_id::DefId;
@@ -23,9 +22,6 @@ pub struct QueryStackFrame<I> {
     pub info: I,
 
     pub dep_kind: DepKind,
-    /// This hash is used to deterministically pick
-    /// a query to remove cycles in the parallel compiler.
-    pub hash: Hash64,
     pub def_id: Option<DefId>,
     /// A def-id that is extracted from a `Ty` in a query key
     pub def_id_for_ty_in_cycle: Option<DefId>,
@@ -36,18 +32,16 @@ impl<'tcx> QueryStackFrame<QueryStackDeferred<'tcx>> {
     pub fn new(
         info: QueryStackDeferred<'tcx>,
         dep_kind: DepKind,
-        hash: Hash64,
         def_id: Option<DefId>,
         def_id_for_ty_in_cycle: Option<DefId>,
     ) -> Self {
-        Self { info, def_id, dep_kind, hash, def_id_for_ty_in_cycle }
+        Self { info, def_id, dep_kind, def_id_for_ty_in_cycle }
     }
 
     pub fn lift(&self) -> QueryStackFrame<QueryStackFrameExtra> {
         QueryStackFrame {
             info: self.info.extract(),
             dep_kind: self.dep_kind,
-            hash: self.hash,
             def_id: self.def_id,
             def_id_for_ty_in_cycle: self.def_id_for_ty_in_cycle,
         }

compiler/rustc_query_impl/Cargo.toml

@@ -9,7 +9,6 @@ measureme = "12.0.1"
 rustc_abi = { path = "../rustc_abi" }
 rustc_data_structures = { path = "../rustc_data_structures" }
 rustc_errors = { path = "../rustc_errors" }
-rustc_hashes = { path = "../rustc_hashes" }
 rustc_hir = { path = "../rustc_hir" }
 rustc_index = { path = "../rustc_index" }
 rustc_macros = { path = "../rustc_macros" }

compiler/rustc_query_impl/src/job.rs

@@ -209,27 +209,6 @@ fn connected_to_root<'tcx>(
     visit_waiters(job_map, query, |_, successor| connected_to_root(job_map, successor, visited))
 }
 
-// Deterministically pick an query from a list
-fn pick_query<'a, 'tcx, T, F>(job_map: &QueryJobMap<'tcx>, queries: &'a [T], f: F) -> &'a T
-where
-    F: Fn(&T) -> (Span, QueryJobId),
-{
-    // Deterministically pick an entry point
-    // FIXME: Sort this instead
-    queries
-        .iter()
-        .min_by_key(|v| {
-            let (span, query) = f(v);
-            let hash = job_map.frame_of(query).hash;
-            // Prefer entry points which have valid spans for nicer error messages
-            // We add an integer to the tuple ensuring that entry points
-            // with valid spans are picked first
-            let span_cmp = if span == DUMMY_SP { 1 } else { 0 };
-            (span_cmp, hash)
-        })
-        .unwrap()
-}
-
 /// Looks for query cycles starting from the last query in `jobs`.
 /// If a cycle is found, all queries in the cycle is removed from `jobs` and
 /// the function return true.
@@ -263,48 +242,56 @@ fn remove_cycle<'tcx>(
             }
         }
 
+        struct EntryPoint {
+            query_in_cycle: QueryJobId,
+            waiter: Option<(Span, QueryJobId)>,
+        }
+
         // Find the queries in the cycle which are
         // connected to queries outside the cycle
         let entry_points = stack
             .iter()
-            .filter_map(|&(span, query)| {
-                if job_map.parent_of(query).is_none() {
+            .filter_map(|&(_, query_in_cycle)| {
+                if job_map.parent_of(query_in_cycle).is_none() {
                     // This query is connected to the root (it has no query parent)
-                    Some((span, query, None))
+                    Some(EntryPoint { query_in_cycle, waiter: None })
                 } else {
-                    let mut waiters = Vec::new();
-                    // Find all the direct waiters who lead to the root
-                    let _ = visit_waiters(job_map, query, |span, waiter| {
+                    let mut waiter_on_cycle = None;
+                    // Find a direct waiter who leads to the root
+                    let _ = visit_waiters(job_map, query_in_cycle, |span, waiter| {
                         // Mark all the other queries in the cycle as already visited
                         let mut visited = FxHashSet::from_iter(stack.iter().map(|q| q.1));
 
                         if connected_to_root(job_map, waiter, &mut visited).is_break() {
-                            waiters.push((span, waiter));
+                            waiter_on_cycle = Some((span, waiter));
+                            ControlFlow::Break(None)
+                        } else {
+                            ControlFlow::Continue(())
                         }
-
-                        ControlFlow::Continue(())
                     });
-                    if waiters.is_empty() {
-                        None
-                    } else {
-                        // Deterministically pick one of the waiters to show to the user
-                        let waiter = *pick_query(job_map, &waiters, |s| *s);
-                        Some((span, query, Some(waiter)))
-                    }
+
+                    waiter_on_cycle.map(|waiter_on_cycle| EntryPoint {
+                        query_in_cycle,
+                        waiter: Some(waiter_on_cycle),
+                    })
                 }
             })
-            .collect::<Vec<(Span, QueryJobId, Option<(Span, QueryJobId)>)>>();
+            .collect::<Vec<EntryPoint>>();
 
-        // Deterministically pick an entry point
-        let (_, entry_point, usage) = pick_query(job_map, &entry_points, |e| (e.0, e.1));
+        // Pick an entry point, preferring ones with waiters
+        let entry_point = entry_points
+            .iter()
+            .find(|entry_point| entry_point.waiter.is_some())
+            .unwrap_or(&entry_points[0]);
 
         // Shift the stack so that our entry point is first
-        let entry_point_pos = stack.iter().position(|(_, query)| query == entry_point);
+        let entry_point_pos =
+            stack.iter().position(|(_, query)| *query == entry_point.query_in_cycle);
         if let Some(pos) = entry_point_pos {
             stack.rotate_left(pos);
         }
 
-        let usage = usage.map(|(span, job)| (span, job_map.frame_of(job).clone()));
+        let usage = entry_point.waiter.map(|(span, job)| (span, job_map.frame_of(job).clone()));
 
         // Create the cycle error
         let error = CycleError {

compiler/rustc_query_impl/src/plumbing.rs

@@ -4,10 +4,8 @@
 
 use std::num::NonZero;
 
-use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_data_structures::unord::UnordMap;
-use rustc_hashes::Hash64;
 use rustc_hir::def_id::DefId;
 use rustc_hir::limit::Limit;
 use rustc_index::Idx;
@@ -319,18 +317,11 @@ where
 {
     let kind = vtable.dep_kind;
 
-    let hash = tcx.with_stable_hashing_context(|mut hcx| {
-        let mut hasher = StableHasher::new();
-        kind.as_usize().hash_stable(&mut hcx, &mut hasher);
-        key.hash_stable(&mut hcx, &mut hasher);
-        hasher.finish::<Hash64>()
-    });
-
     let def_id: Option<DefId> = key.key_as_def_id();
     let def_id_for_ty_in_cycle: Option<DefId> = key.def_id_for_ty_in_cycle();
 
     let info = QueryStackDeferred::new((tcx, vtable, key), mk_query_stack_frame_extra);
-    QueryStackFrame::new(info, kind, hash, def_id, def_id_for_ty_in_cycle)
+    QueryStackFrame::new(info, kind, def_id, def_id_for_ty_in_cycle)
 }
 
 pub(crate) fn encode_query_results<'a, 'tcx, Q, C: QueryCache, const FLAGS: QueryFlags>(