Auto merge of #110069 - ndrewxie:issue-104212-fix, r=cjgillot

Switched provisional evaluation cache map to FxIndexMap, and replaced map.drain_filter with map.retain

Switching ProvisionalEvaluationCache's map field from FxHashMap to FxIndexMap was previously blocked because doing so caused performance regressions that could be mitigated by the stabilization of drain_filter for FxIndexMap (#104212). However, the only use of drain_filter can be replaced with a retain, so I made the modification and put in a PR to see if this causes a performance regression as well.

This PR is part of a broader effort (#84447) of removing iteration through FxHashMaps, as the iteration order is unstable and can cause issues in incremental compilation.
This commit is contained in:
bors 2023-04-08 11:20:58 +00:00
commit 66f8dd1255

View File

@ -2,12 +2,6 @@
//!
//! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/traits/resolution.html#selection
// FIXME: The `map` field in ProvisionalEvaluationCache should be changed to
// a `FxIndexMap` to avoid query instability, but right now it causes a perf regression. This would be
// fixed or at least lightened by the addition of the `drain_filter` method to `FxIndexMap`
// Relevant: https://github.com/rust-lang/rust/pull/103723 and https://github.com/bluss/indexmap/issues/242
#![allow(rustc::potential_query_instability)]
use self::EvaluationResult::*;
use self::SelectionCandidate::*;
@ -32,8 +26,7 @@
use crate::traits::project::ProjectionCacheKeyExt;
use crate::traits::ProjectionCacheKey;
use crate::traits::Unimplemented;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::Diagnostic;
use rustc_hir as hir;
@ -2782,7 +2775,7 @@ struct ProvisionalEvaluationCache<'tcx> {
/// - then we determine that `E` is in error -- we will then clear
/// all cache values whose DFN is >= 4 -- in this case, that
/// means the cached value for `F`.
map: RefCell<FxHashMap<ty::PolyTraitPredicate<'tcx>, ProvisionalEvaluation>>,
map: RefCell<FxIndexMap<ty::PolyTraitPredicate<'tcx>, ProvisionalEvaluation>>,
/// The stack of args that we assume to be true because a `WF(arg)` predicate
/// is on the stack above (and because of wellformedness is coinductive).
@ -2930,12 +2923,13 @@ fn on_failure(&self, dfn: usize) {
/// have a performance impact in practice.
fn on_completion(&self, dfn: usize) {
debug!(?dfn, "on_completion");
for (fresh_trait_pred, eval) in
self.map.borrow_mut().drain_filter(|_k, eval| eval.from_dfn >= dfn)
{
debug!(?fresh_trait_pred, ?eval, "on_completion");
}
self.map.borrow_mut().retain(|fresh_trait_pred, eval| {
if eval.from_dfn >= dfn {
debug!(?fresh_trait_pred, ?eval, "on_completion");
return false;
}
true
});
}
}