Remove deterministic picking from query cycle handling

Cargo.lock

@@ -4517,7 +4517,6 @@ dependencies = [
  "measureme",
  "rustc_data_structures",
  "rustc_errors",
- "rustc_hashes",
  "rustc_hir",
  "rustc_index",
  "rustc_macros",
@@ -4538,7 +4537,6 @@ dependencies = [
  "rustc_data_structures",
  "rustc_errors",
  "rustc_feature",
- "rustc_hashes",
  "rustc_hir",
  "rustc_index",
  "rustc_macros",

compiler/rustc_query_impl/Cargo.toml

@@ -8,7 +8,6 @@ edition = "2024"
 measureme = "12.0.1"
 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/plumbing.rs

@@ -5,10 +5,8 @@
 use std::num::NonZero;
 
 use rustc_data_structures::jobserver::Proxy;
-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;
@@ -26,7 +24,6 @@ use rustc_middle::ty::print::with_reduced_queries;
 use rustc_middle::ty::tls::{self, ImplicitCtxt};
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_query_system::dep_graph::{DepNodeKey, FingerprintStyle, HasDepContext};
-use rustc_query_system::ich::StableHashingContext;
 use rustc_query_system::query::{
     QueryCache, QueryContext, QueryDispatcher, QueryJobId, QueryMap, QuerySideEffect,
     QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra, force_query,
@@ -358,22 +355,15 @@ pub(crate) fn create_deferred_query_stack_frame<'tcx, Cache>(
 ) -> QueryStackFrame<QueryStackDeferred<'tcx>>
 where
     Cache: QueryCache,
-    Cache::Key: Key + DynSend + DynSync + for<'a> HashStable<StableHashingContext<'a>> + 'tcx,
+    Cache::Key: Key + DynSend + DynSync,
 {
     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>(

compiler/rustc_query_system/Cargo.toml

@@ -11,7 +11,6 @@ rustc_ast = { path = "../rustc_ast" }
 rustc_data_structures = { path = "../rustc_data_structures" }
 rustc_errors = { path = "../rustc_errors" }
 rustc_feature = { path = "../rustc_feature" }
-rustc_hashes = { path = "../rustc_hashes" }
 rustc_hir = { path = "../rustc_hir" }
 rustc_index = { path = "../rustc_index" }
 rustc_macros = { path = "../rustc_macros" }

compiler/rustc_query_system/src/query/job.rs

@@ -377,27 +377,6 @@ fn connected_to_root<'tcx>(
     .is_some()
 }
 
-// Deterministically pick an query from a list
-fn pick_query<'a, 'tcx, T, F>(query_map: &QueryMap<'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 = query.frame(query_map).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.
@@ -431,48 +410,58 @@ 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 query.parent(query_map).is_none() {
+            .filter_map(|&(_, query_in_cycle)| {
+                if query_in_cycle.parent(query_map).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
-                    visit_waiters(query_map, query, |span, waiter| {
+                    let mut waiter_on_cycle = None;
+                    // Find a direct waiter who leads to the root
+                    visit_waiters(query_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(query_map, waiter, &mut visited) {
-                            waiters.push((span, waiter));
+                            waiter_on_cycle = Some((span, waiter));
+                            Some(None)
+                        } else {
+                            None
                         }
-
-                        None
                     });
-                    if waiters.is_empty() {
-                        None
-                    } else {
-                        // Deterministically pick one of the waiters to show to the user
-                        let waiter = *pick_query(query_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(query_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.first().unwrap());
 
         // 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.as_ref().map(|(span, query)| (*span, query.frame(query_map)));
+        let usage =
+            entry_point.waiter.as_ref().map(|(span, query)| (*span, query.frame(query_map)));
 
         // Create the cycle error
         let error = CycleError {

compiler/rustc_query_system/src/query/mod.rs

@@ -6,7 +6,6 @@ use std::sync::Arc;
 use rustc_data_structures::jobserver::Proxy;
 use rustc_data_structures::sync::{DynSend, DynSync};
 use rustc_errors::DiagInner;
-use rustc_hashes::Hash64;
 use rustc_hir::def::DefKind;
 use rustc_macros::{Decodable, Encodable};
 use rustc_span::Span;
@@ -50,9 +49,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.
-    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>,
@@ -63,18 +59,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 }
     }
 
     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,
         }