Skip to content

Commit

Permalink
Auto merge of #69294 - eddyb:trait-cache-streamline, r=<try>
Browse files Browse the repository at this point in the history 
[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
  • Loading branch information
@bors
bors committed Feb 19, 2020
2 parents 7710ae0 + 6a51d66 commit a540ddc
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 89 deletions.
7 changes: 5 additions & 2 deletions src/librustc/traits/select.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>>>,
>,
>,
Expand Down Expand Up @@ -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>,
>,
>,
}

Expand Down
169 changes: 82 additions & 87 deletions src/librustc_infer/traits/select.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -982,6 +976,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
cache_fresh_trait_pred,
dep_node,
candidate.clone(),
stack.obligation.cause.span,
);
candidate
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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
Expand Down Expand Up @@ -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>(
Expand Down

0 comments on commit a540ddc

Please sign in to comment.