Auto merge of #69294 - eddyb:trait-cache-streamline, r=<try>

[WIP] traits/select: use global vs per-infcx caches more uniformly.

~~*Note: this is based on an older `master` to avoid perf interference before #67953 (comment) is resolved.*~~ **EDIT**: sadly not workable due #69294 (comment).

So far this PR only has the first couple steps towards making the decision of which cache ("global" vs "per-`InferCtxt`") to use for trait evaluation/selection, only once (at the time of checking the cache).

The goal here is to actually make per-`InferCtxt` caches not track `DepNode`s, and maybe even enforce that once `SelectionContext::in_task` is entered, the `InferCtxt` is effectively unused.

My assumption is that if you *need* inference variables in your cache key (i.e. `ParamEnv` and/or `PolyTraitPredicate`) to ever end up doing anything "non-global", and you can't get there from a "global cache key" (which would still use `DepNode`s and `in_task` etc.).

Perhaps another path forward is to redirect "global cache keys" to a query, but I'm not sure how that would handle cycles (badly?), and it feels like stepping on Chalk's toes.

r? @nikomatsakis cc @rust-lang/wg-traits @Zoxc @michaelwoerister

Author: bors

src/librustc/traits/select.rs

@@ -17,7 +17,7 @@ use rustc_hir::def_id::DefId;
 pub struct SelectionCache<'tcx> {
     pub hashmap: Lock<
         FxHashMap<
-            ty::ParamEnvAnd<'tcx, ty::TraitRef<'tcx>>,
+            ty::ParamEnvAnd<'tcx, ty::PolyTraitPredicate<'tcx>>,
             WithDepNode<SelectionResult<'tcx, SelectionCandidate<'tcx>>>,
         >,
     >,
@@ -261,7 +261,10 @@ impl<'tcx> From<OverflowError> for SelectionError<'tcx> {
 #[derive(Clone, Default)]
 pub struct EvaluationCache<'tcx> {
     pub hashmap: Lock<
-        FxHashMap<ty::ParamEnvAnd<'tcx, ty::PolyTraitRef<'tcx>>, WithDepNode<EvaluationResult>>,
+        FxHashMap<
+            ty::ParamEnvAnd<'tcx, ty::PolyTraitPredicate<'tcx>>,
+            WithDepNode<EvaluationResult>,
+        >,
     >,
 }
 

src/librustc_infer/traits/select.rs

@@ -835,18 +835,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         trait_ref: ty::PolyTraitRef<'tcx>,
     ) -> Option<EvaluationResult> {
         let tcx = self.tcx();
-        if self.can_use_global_caches(param_env) {
-            let cache = tcx.evaluation_cache.hashmap.borrow();
-            if let Some(cached) = cache.get(&param_env.and(trait_ref)) {
-                return Some(cached.get(tcx));
-            }
-        }
-        self.infcx
-            .evaluation_cache
-            .hashmap
-            .borrow()
-            .get(&param_env.and(trait_ref))
-            .map(|v| v.get(tcx))
+        // FIXME(eddyb) pass in the `ty::PolyTraitPredicate` instead.
+        let predicate = trait_ref.map_bound(|trait_ref| ty::TraitPredicate { trait_ref });
+        // FIXME(eddyb) reuse the key between checking the cache and inserting.
+        let cache_key = param_env.and(predicate);
+        let cache = if self.can_use_global_caches(&cache_key) {
+            &tcx.evaluation_cache
+        } else {
+            &self.infcx.evaluation_cache
+        };
+
+        cache.hashmap.borrow().get(&cache_key).map(|v| v.get(tcx))
     }
 
     fn insert_evaluation_cache(
@@ -862,31 +861,26 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             return;
         }
 
-        if self.can_use_global_caches(param_env) {
-            if !trait_ref.has_local_value() {
-                debug!(
-                    "insert_evaluation_cache(trait_ref={:?}, candidate={:?}) global",
-                    trait_ref, result,
-                );
-                // This may overwrite the cache with the same value
-                // FIXME: Due to #50507 this overwrites the different values
-                // This should be changed to use HashMapExt::insert_same
-                // when that is fixed
-                self.tcx()
-                    .evaluation_cache
-                    .hashmap
-                    .borrow_mut()
-                    .insert(param_env.and(trait_ref), WithDepNode::new(dep_node, result));
-                return;
-            }
-        }
+        // FIXME(eddyb) pass in the `ty::PolyTraitPredicate` instead.
+        let predicate = trait_ref.map_bound(|trait_ref| ty::TraitPredicate { trait_ref });
+        // FIXME(eddyb) reuse the key between checking the cache and inserting.
+        let cache_key = param_env.and(predicate);
+        let cache = if self.can_use_global_caches(&cache_key) {
+            debug!(
+                "insert_evaluation_cache(trait_ref={:?}, candidate={:?}) global",
+                trait_ref, result,
+            );
+            // This may overwrite the cache with the same value
+            // FIXME: Due to #50507 this overwrites the different values
+            // This should be changed to use HashMapExt::insert_same
+            // when that is fixed
+            &self.tcx().evaluation_cache
+        } else {
+            debug!("insert_evaluation_cache(trait_ref={:?}, candidate={:?})", trait_ref, result,);
+            &self.infcx.evaluation_cache
+        };
 
-        debug!("insert_evaluation_cache(trait_ref={:?}, candidate={:?})", trait_ref, result,);
-        self.infcx
-            .evaluation_cache
-            .hashmap
-            .borrow_mut()
-            .insert(param_env.and(trait_ref), WithDepNode::new(dep_node, result));
+        cache.hashmap.borrow_mut().insert(cache_key, WithDepNode::new(dep_node, result));
     }
 
     /// For various reasons, it's possible for a subobligation
@@ -961,7 +955,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         debug_assert!(!stack.obligation.predicate.has_escaping_bound_vars());
 
         if let Some(c) =
-            self.check_candidate_cache(stack.obligation.param_env, &cache_fresh_trait_pred)
+            self.check_candidate_cache(stack.obligation.param_env, cache_fresh_trait_pred)
         {
             debug!("CACHE HIT: SELECT({:?})={:?}", cache_fresh_trait_pred, c);
             return c;
@@ -982,6 +976,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             cache_fresh_trait_pred,
             dep_node,
             candidate.clone(),
+            stack.obligation.cause.span,
         );
         candidate
     }
@@ -1218,13 +1213,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     }
 
     /// Returns `true` if the global caches can be used.
-    /// Do note that if the type itself is not in the
-    /// global tcx, the local caches will be used.
-    fn can_use_global_caches(&self, param_env: ty::ParamEnv<'tcx>) -> bool {
-        // If there are any e.g. inference variables in the `ParamEnv`, then we
-        // always use a cache local to this particular scope. Otherwise, we
-        // switch to a global cache.
-        if param_env.has_local_value() {
+    fn can_use_global_caches(
+        &self,
+        cache_key: &ty::ParamEnvAnd<'tcx, ty::PolyTraitPredicate<'tcx>>,
+    ) -> bool {
+        // If there are any e.g. inference variables in the `ParamEnv` or predicate,
+        // then we always use a cache local to this particular inference context.
+        // Otherwise, we switch to a global cache.
+        if cache_key.has_local_value() {
             return false;
         }
 
@@ -1246,22 +1242,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn check_candidate_cache(
         &mut self,
         param_env: ty::ParamEnv<'tcx>,
-        cache_fresh_trait_pred: &ty::PolyTraitPredicate<'tcx>,
+        cache_fresh_trait_pred: ty::PolyTraitPredicate<'tcx>,
     ) -> Option<SelectionResult<'tcx, SelectionCandidate<'tcx>>> {
         let tcx = self.tcx();
-        let trait_ref = &cache_fresh_trait_pred.skip_binder().trait_ref;
-        if self.can_use_global_caches(param_env) {
-            let cache = tcx.selection_cache.hashmap.borrow();
-            if let Some(cached) = cache.get(&param_env.and(*trait_ref)) {
-                return Some(cached.get(tcx));
-            }
-        }
-        self.infcx
-            .selection_cache
-            .hashmap
-            .borrow()
-            .get(&param_env.and(*trait_ref))
-            .map(|v| v.get(tcx))
+        // FIXME(eddyb) reuse the key between checking the cache and inserting.
+        let cache_key = param_env.and(cache_fresh_trait_pred);
+        let cache = if self.can_use_global_caches(&cache_key) {
+            &tcx.selection_cache
+        } else {
+            &self.infcx.selection_cache
+        };
+
+        cache.hashmap.borrow().get(&cache_key).map(|v| v.get(tcx))
     }
 
     /// Determines whether can we safely cache the result
@@ -1298,47 +1290,50 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         cache_fresh_trait_pred: ty::PolyTraitPredicate<'tcx>,
         dep_node: DepNodeIndex,
         candidate: SelectionResult<'tcx, SelectionCandidate<'tcx>>,
+        span: rustc_span::Span,
     ) {
         let tcx = self.tcx();
-        let trait_ref = cache_fresh_trait_pred.skip_binder().trait_ref;
 
         if !self.can_cache_candidate(&candidate) {
             debug!(
-                "insert_candidate_cache(trait_ref={:?}, candidate={:?} -\
+                "insert_candidate_cache(predicate={:?}, candidate={:?} -\
                  candidate is not cacheable",
-                trait_ref, candidate
+                cache_fresh_trait_pred, candidate
             );
             return;
         }
 
-        if self.can_use_global_caches(param_env) {
-            if let Err(Overflow) = candidate {
-                // Don't cache overflow globally; we only produce this in certain modes.
-            } else if !trait_ref.has_local_value() {
-                if !candidate.has_local_value() {
-                    debug!(
-                        "insert_candidate_cache(trait_ref={:?}, candidate={:?}) global",
-                        trait_ref, candidate,
-                    );
-                    // This may overwrite the cache with the same value.
-                    tcx.selection_cache
-                        .hashmap
-                        .borrow_mut()
-                        .insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate));
-                    return;
-                }
-            }
+        // HACK(eddyb) never cache overflow (this check used to be global-only).
+        if let Err(Overflow) = candidate {
+            return;
         }
 
-        debug!(
-            "insert_candidate_cache(trait_ref={:?}, candidate={:?}) local",
-            trait_ref, candidate,
-        );
-        self.infcx
-            .selection_cache
-            .hashmap
-            .borrow_mut()
-            .insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate));
+        // FIXME(eddyb) reuse the key between checking the cache and inserting.
+        let cache_key = param_env.and(cache_fresh_trait_pred);
+        let cache = if self.can_use_global_caches(&cache_key) {
+            if candidate.has_local_value() {
+                span_bug!(
+                    span,
+                    "selecting inference-free `{:?}` resulted in `{:?}`?!",
+                    cache_fresh_trait_pred,
+                    candidate,
+                );
+            }
+            debug!(
+                "insert_candidate_cache(predicate={:?}, candidate={:?}) global",
+                cache_fresh_trait_pred, candidate,
+            );
+            // This may overwrite the cache with the same value.
+            &tcx.selection_cache
+        } else {
+            debug!(
+                "insert_candidate_cache(predicate={:?}, candidate={:?}) local",
+                cache_fresh_trait_pred, candidate,
+            );
+            &self.infcx.selection_cache
+        };
+
+        cache.hashmap.borrow_mut().insert(cache_key, WithDepNode::new(dep_node, candidate));
     }
 
     fn assemble_candidates<'o>(