From 9ee7a15e5ff050af32fd968875a61880f5e7f3dd Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 19 Feb 2020 19:15:34 +0200 Subject: [PATCH 1/2] traits/select: use global vs per-infcx caches more uniformly. --- src/librustc_infer/traits/select.rs | 126 +++++++++++++--------------- 1 file changed, 58 insertions(+), 68 deletions(-) diff --git a/src/librustc_infer/traits/select.rs b/src/librustc_infer/traits/select.rs index 371268b5ee471..9648ed63f158b 100644 --- a/src/librustc_infer/traits/select.rs +++ b/src/librustc_infer/traits/select.rs @@ -835,18 +835,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { trait_ref: ty::PolyTraitRef<'tcx>, ) -> Option { 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(¶m_env.and(trait_ref)) { - return Some(cached.get(tcx)); - } - } - self.infcx - .evaluation_cache - .hashmap - .borrow() - .get(¶m_env.and(trait_ref)) - .map(|v| v.get(tcx)) + let cache = if self.can_use_global_caches(param_env) && !trait_ref.has_local_value() { + &tcx.evaluation_cache + } else { + &self.infcx.evaluation_cache + }; + + cache.hashmap.borrow().get(¶m_env.and(trait_ref)).map(|v| v.get(tcx)) } fn insert_evaluation_cache( @@ -862,28 +857,22 @@ 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; - } - } + let cache = if self.can_use_global_caches(param_env) && !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 + } 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 + cache .hashmap .borrow_mut() .insert(param_env.and(trait_ref), WithDepNode::new(dep_node, result)); @@ -982,6 +971,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { cache_fresh_trait_pred, dep_node, candidate.clone(), + stack.obligation.cause.span, ); candidate } @@ -1250,18 +1240,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ) -> Option>> { 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(¶m_env.and(*trait_ref)) { - return Some(cached.get(tcx)); - } - } - self.infcx - .selection_cache - .hashmap - .borrow() - .get(¶m_env.and(*trait_ref)) - .map(|v| v.get(tcx)) + let cache = if self.can_use_global_caches(param_env) && !trait_ref.has_local_value() { + &tcx.selection_cache + } else { + &self.infcx.selection_cache + }; + + cache.hashmap.borrow().get(¶m_env.and(*trait_ref)).map(|v| v.get(tcx)) } /// Determines whether can we safely cache the result @@ -1298,6 +1283,7 @@ 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; @@ -1311,31 +1297,35 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { 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 + let cache = if self.can_use_global_caches(param_env) && !trait_ref.has_local_value() { + if candidate.has_local_value() { + span_bug!( + span, + "selecting inference-free `{}` resulted in `{:?}`?!", + trait_ref, + candidate, + ); + } + debug!( + "insert_candidate_cache(trait_ref={:?}, candidate={:?}) global", + trait_ref, candidate, + ); + // This may overwrite the cache with the same value. + &tcx.selection_cache + } else { + debug!( + "insert_candidate_cache(trait_ref={:?}, candidate={:?}) local", + trait_ref, candidate, + ); + &self.infcx.selection_cache + }; + + cache .hashmap .borrow_mut() .insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate)); From 6a51d66ba80a97ef9d5fff61df60f1f074e0abe4 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 19 Feb 2020 19:20:49 +0200 Subject: [PATCH 2/2] traits/select: pass the whole cache key to can_use_global_caches. --- src/librustc/traits/select.rs | 7 ++- src/librustc_infer/traits/select.rs | 71 +++++++++++++++-------------- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index ac3d0049c0c7c..e543321e90c5f 100644 --- a/src/librustc/traits/select.rs +++ b/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>>, >, >, @@ -261,7 +261,10 @@ impl<'tcx> From for SelectionError<'tcx> { #[derive(Clone, Default)] pub struct EvaluationCache<'tcx> { pub hashmap: Lock< - FxHashMap>, WithDepNode>, + FxHashMap< + ty::ParamEnvAnd<'tcx, ty::PolyTraitPredicate<'tcx>>, + WithDepNode, + >, >, } diff --git a/src/librustc_infer/traits/select.rs b/src/librustc_infer/traits/select.rs index 9648ed63f158b..84f9d79af0eb0 100644 --- a/src/librustc_infer/traits/select.rs +++ b/src/librustc_infer/traits/select.rs @@ -835,13 +835,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { trait_ref: ty::PolyTraitRef<'tcx>, ) -> Option { let tcx = self.tcx(); - let cache = if self.can_use_global_caches(param_env) && !trait_ref.has_local_value() { + // 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(¶m_env.and(trait_ref)).map(|v| v.get(tcx)) + cache.hashmap.borrow().get(&cache_key).map(|v| v.get(tcx)) } fn insert_evaluation_cache( @@ -857,7 +861,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return; } - let cache = if self.can_use_global_caches(param_env) && !trait_ref.has_local_value() { + // 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, @@ -872,10 +880,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { &self.infcx.evaluation_cache }; - 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 @@ -950,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; @@ -1208,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; } @@ -1236,17 +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>> { let tcx = self.tcx(); - let trait_ref = &cache_fresh_trait_pred.skip_binder().trait_ref; - let cache = if self.can_use_global_caches(param_env) && !trait_ref.has_local_value() { + // 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(¶m_env.and(*trait_ref)).map(|v| v.get(tcx)) + cache.hashmap.borrow().get(&cache_key).map(|v| v.get(tcx)) } /// Determines whether can we safely cache the result @@ -1286,13 +1293,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, '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; } @@ -1302,33 +1308,32 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return; } - let cache = if self.can_use_global_caches(param_env) && !trait_ref.has_local_value() { + // 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 `{:?}`?!", - trait_ref, + "selecting inference-free `{:?}` resulted in `{:?}`?!", + cache_fresh_trait_pred, candidate, ); } debug!( - "insert_candidate_cache(trait_ref={:?}, candidate={:?}) global", - trait_ref, candidate, + "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(trait_ref={:?}, candidate={:?}) local", - trait_ref, candidate, + "insert_candidate_cache(predicate={:?}, candidate={:?}) local", + cache_fresh_trait_pred, candidate, ); &self.infcx.selection_cache }; - cache - .hashmap - .borrow_mut() - .insert(param_env.and(trait_ref), WithDepNode::new(dep_node, candidate)); + cache.hashmap.borrow_mut().insert(cache_key, WithDepNode::new(dep_node, candidate)); } fn assemble_candidates<'o>(