Auto merge of rust-lang#118824 - aliemjay:perf-region-cons, r=<try>

use Vec for region constraints instead of BTreeMap

~1% perf gain

Diagnostic regressions need more investigation.

r? `@ghost`

compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs

@@ -137,6 +137,9 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
     ) -> LexicalRegionResolutions<'tcx> {
         let mut var_data = self.construct_var_data();
 
+        self.data.constraints.sort_by_key(|(constraint, _)| *constraint);
+        self.data.constraints.dedup_by_key(|(constraint, _)| *constraint);
+
         if cfg!(debug_assertions) {
             self.dump_constraints();
         }
@@ -183,7 +186,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
         let mut constraints = IndexVec::from_elem(Vec::new(), &var_values.values);
         // Tracks the changed region vids.
         let mut changes = Vec::new();
-        for constraint in self.data.constraints.keys() {
+        for (constraint, _) in &self.data.constraints {
             match *constraint {
                 Constraint::RegSubVar(a_region, b_vid) => {
                     let b_data = var_values.value_mut(b_vid);
@@ -678,7 +681,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
         let dummy_source = graph.add_node(());
         let dummy_sink = graph.add_node(());
 
-        for constraint in self.data.constraints.keys() {
+        for (constraint, _) in &self.data.constraints {
             match *constraint {
                 Constraint::VarSubVar(a_id, b_id) => {
                     graph.add_edge(NodeIndex(a_id.index()), NodeIndex(b_id.index()), *constraint);
@@ -885,9 +888,11 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
                     }
 
                     Constraint::RegSubVar(region, _) | Constraint::VarSubReg(_, region) => {
+                        let constraint_idx =
+                            this.constraints.binary_search_by_key(&edge.data, |(c, _)| *c).unwrap();
                         state.result.push(RegionAndOrigin {
                             region,
-                            origin: this.constraints.get(&edge.data).unwrap().clone(),
+                            origin: this.constraints[constraint_idx].1.clone(),
                         });
                     }
 

compiler/rustc_infer/src/infer/mod.rs

@@ -18,7 +18,6 @@ use crate::traits::{self, ObligationCause, PredicateObligations, TraitEngine, Tr
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::sync::Lrc;
-use rustc_data_structures::undo_log::Rollback;
 use rustc_data_structures::unify as ut;
 use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed};
 use rustc_hir::def_id::{DefId, LocalDefId};
@@ -919,7 +918,7 @@ impl<'tcx> InferCtxt<'tcx> {
         self.inner
             .borrow_mut()
             .unwrap_region_constraints()
-            .region_constraints_added_in_snapshot(&snapshot.undo_snapshot)
+            .region_constraints_added_in_snapshot(&snapshot.region_constraints_snapshot)
     }
 
     pub fn opaque_types_added_in_snapshot(&self, snapshot: &CombinedSnapshot<'tcx>) -> bool {

compiler/rustc_infer/src/infer/region_constraints/leak_check.rs

@@ -412,26 +412,15 @@ impl<'tcx> MiniGraph<'tcx> {
         };
 
         if let Some(snapshot) = only_consider_snapshot {
-            for undo_entry in
-                region_constraints.undo_log.region_constraints_in_snapshot(&snapshot.undo_snapshot)
-            {
-                match undo_entry {
-                    AddConstraint(constraint) => {
-                        each_constraint(constraint);
-                    }
-                    &AddVerify(i) => span_bug!(
-                        region_constraints.data().verifys[i].origin.span(),
-                        "we never add verifications while doing higher-ranked things",
-                    ),
-                    &AddCombination(..) | &AddVar(..) => {}
-                }
-            }
+            region_constraints
+                .region_constraints_in_snapshot(&snapshot.region_constraints_snapshot)
+                .for_each(each_constraint);
         } else {
             region_constraints
                 .data()
                 .constraints
-                .keys()
-                .for_each(|constraint| each_constraint(constraint));
+                .iter()
+                .for_each(|(constraint, _)| each_constraint(constraint));
         }
     }
 

compiler/rustc_infer/src/infer/region_constraints/mod.rs

@@ -1,13 +1,10 @@
 //! See `README.md`.
 
 use self::CombineMapType::*;
-use self::UndoLog::*;
 
-use super::{
-    InferCtxtUndoLogs, MiscVariable, RegionVariableOrigin, Rollback, Snapshot, SubregionOrigin,
-};
+use super::{InferCtxtUndoLogs, MiscVariable, RegionVariableOrigin, SubregionOrigin};
 
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::intern::Interned;
 use rustc_data_structures::sync::Lrc;
 use rustc_data_structures::undo_log::UndoLogs;
@@ -20,7 +17,6 @@ use rustc_middle::ty::{ReBound, ReVar};
 use rustc_middle::ty::{Region, RegionVid};
 use rustc_span::Span;
 
-use std::collections::BTreeMap;
 use std::ops::Range;
 use std::{cmp, fmt, mem};
 
@@ -90,7 +86,7 @@ pub type VarInfos = IndexVec<RegionVid, RegionVariableInfo>;
 pub struct RegionConstraintData<'tcx> {
     /// Constraints of the form `A <= B`, where either `A` or `B` can
     /// be a region variable (or neither, as it happens).
-    pub constraints: BTreeMap<Constraint<'tcx>, SubregionOrigin<'tcx>>,
+    pub constraints: Vec<(Constraint<'tcx>, SubregionOrigin<'tcx>)>,
 
     /// Constraints of the form `R0 member of [R1, ..., Rn]`, meaning that
     /// `R0` must be equal to one of the regions `R1..Rn`. These occur
@@ -267,28 +263,13 @@ pub(crate) struct TwoRegions<'tcx> {
     b: Region<'tcx>,
 }
 
-#[derive(Copy, Clone, PartialEq)]
-pub(crate) enum UndoLog<'tcx> {
-    /// We added `RegionVid`.
-    AddVar(RegionVid),
-
-    /// We added the given `constraint`.
-    AddConstraint(Constraint<'tcx>),
-
-    /// We added the given `verify`.
-    AddVerify(usize),
-
-    /// We added a GLB/LUB "combination variable".
-    AddCombination(CombineMapType, TwoRegions<'tcx>),
-}
-
 #[derive(Copy, Clone, PartialEq)]
 pub(crate) enum CombineMapType {
     Lub,
     Glb,
 }
 
-type CombineMap<'tcx> = FxHashMap<TwoRegions<'tcx>, RegionVid>;
+type CombineMap<'tcx> = FxIndexMap<TwoRegions<'tcx>, RegionVid>;
 
 #[derive(Debug, Clone, Copy)]
 pub struct RegionVariableInfo {
@@ -298,6 +279,11 @@ pub struct RegionVariableInfo {
 
 pub struct RegionSnapshot {
     any_unifications: bool,
+    vars_len: usize,
+    constraints_len: usize,
+    verifys_len: usize,
+    glbs_len: usize,
+    lubs_len: usize,
 }
 
 impl<'tcx> RegionConstraintStorage<'tcx> {
@@ -312,28 +298,6 @@ impl<'tcx> RegionConstraintStorage<'tcx> {
     ) -> RegionConstraintCollector<'a, 'tcx> {
         RegionConstraintCollector { storage: self, undo_log }
     }
-
-    fn rollback_undo_entry(&mut self, undo_entry: UndoLog<'tcx>) {
-        match undo_entry {
-            AddVar(vid) => {
-                self.var_infos.pop().unwrap();
-                assert_eq!(self.var_infos.len(), vid.index());
-            }
-            AddConstraint(ref constraint) => {
-                self.data.constraints.remove(constraint);
-            }
-            AddVerify(index) => {
-                self.data.verifys.pop();
-                assert_eq!(self.data.verifys.len(), index);
-            }
-            AddCombination(Glb, ref regions) => {
-                self.glbs.remove(regions);
-            }
-            AddCombination(Lub, ref regions) => {
-                self.lubs.remove(regions);
-            }
-        }
-    }
 }
 
 impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
@@ -407,12 +371,32 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
 
     pub(super) fn start_snapshot(&mut self) -> RegionSnapshot {
         debug!("RegionConstraintCollector: start_snapshot");
-        RegionSnapshot { any_unifications: self.any_unifications }
+        RegionSnapshot {
+            any_unifications: self.any_unifications,
+            vars_len: self.var_infos.len(),
+            constraints_len: self.data.constraints.len(),
+            verifys_len: self.data.verifys.len(),
+            glbs_len: self.glbs.len(),
+            lubs_len: self.lubs.len(),
+        }
     }
 
     pub(super) fn rollback_to(&mut self, snapshot: RegionSnapshot) {
         debug!("RegionConstraintCollector: rollback_to({:?})", snapshot);
-        self.any_unifications = snapshot.any_unifications;
+        let RegionSnapshot {
+            any_unifications,
+            vars_len,
+            constraints_len,
+            verifys_len,
+            glbs_len,
+            lubs_len,
+        } = snapshot;
+        self.any_unifications = any_unifications;
+        self.var_infos.truncate(vars_len);
+        self.data.constraints.truncate(constraints_len);
+        self.data.verifys.truncate(verifys_len);
+        self.glbs.truncate(glbs_len);
+        self.lubs.truncate(lubs_len);
     }
 
     pub(super) fn new_region_var(
@@ -424,7 +408,6 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
 
         let u_vid = self.unification_table_mut().new_key(UnifiedRegion::new(None));
         assert_eq!(vid, u_vid.vid);
-        self.undo_log.push(AddVar(vid));
         debug!("created new region variable {:?} in {:?} with origin {:?}", vid, universe, origin);
         vid
     }
@@ -442,15 +425,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
     fn add_constraint(&mut self, constraint: Constraint<'tcx>, origin: SubregionOrigin<'tcx>) {
         // cannot add constraints once regions are resolved
         debug!("RegionConstraintCollector: add_constraint({:?})", constraint);
-
-        // never overwrite an existing (constraint, origin) - only insert one if it isn't
-        // present in the map yet. This prevents origins from outside the snapshot being
-        // replaced with "less informative" origins e.g., during calls to `can_eq`
-        let undo_log = &mut self.undo_log;
-        self.storage.data.constraints.entry(constraint).or_insert_with(|| {
-            undo_log.push(AddConstraint(constraint));
-            origin
-        });
+        self.storage.data.constraints.push((constraint, origin));
     }
 
     fn add_verify(&mut self, verify: Verify<'tcx>) {
@@ -464,9 +439,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
             return;
         }
 
-        let index = self.data.verifys.len();
         self.data.verifys.push(verify);
-        self.undo_log.push(AddVerify(index));
     }
 
     pub(super) fn make_eqregion(
@@ -638,6 +611,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
         b: Region<'tcx>,
         origin: SubregionOrigin<'tcx>,
     ) -> Region<'tcx> {
+        //assert!(!UndoLogs::<super::UndoLog<'_>>::in_snapshot(&self.undo_log));
         let vars = TwoRegions { a, b };
         if let Some(&c) = self.combine_map(t).get(&vars) {
             return ty::Region::new_var(tcx, c);
@@ -647,7 +621,6 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
         let c_universe = cmp::max(a_universe, b_universe);
         let c = self.new_region_var(c_universe, MiscVariable(origin.span()));
         self.combine_map(t).insert(vars, c);
-        self.undo_log.push(AddCombination(t, vars));
         let new_r = ty::Region::new_var(tcx, c);
         for old_r in [a, b] {
             match t {
@@ -686,10 +659,16 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
     }
 
     /// See `InferCtxt::region_constraints_added_in_snapshot`.
-    pub fn region_constraints_added_in_snapshot(&self, mark: &Snapshot<'tcx>) -> bool {
-        self.undo_log
-            .region_constraints_in_snapshot(mark)
-            .any(|&elt| matches!(elt, AddConstraint(_)))
+    pub fn region_constraints_added_in_snapshot(&self, snapshot: &RegionSnapshot) -> bool {
+        self.data.constraints.len() != snapshot.constraints_len
+    }
+
+    fn region_constraints_in_snapshot(
+        &self,
+        snapshot: &RegionSnapshot,
+    ) -> impl Iterator<Item = &Constraint<'tcx>> {
+        assert_eq!(snapshot.verifys_len, self.data.verifys.len());
+        self.data.constraints[snapshot.constraints_len..].iter().map(|(constraint, _)| constraint)
     }
 
     #[inline]
@@ -774,9 +753,3 @@ impl<'tcx> RegionConstraintData<'tcx> {
         constraints.is_empty() && member_constraints.is_empty() && verifys.is_empty()
     }
 }
-
-impl<'tcx> Rollback<UndoLog<'tcx>> for RegionConstraintStorage<'tcx> {
-    fn reverse(&mut self, undo: UndoLog<'tcx>) {
-        self.rollback_undo_entry(undo)
-    }
-}

compiler/rustc_infer/src/infer/undo_log.rs

@@ -7,7 +7,7 @@ use rustc_middle::infer::unify_key::{ConstVidKey, EffectVidKey, RegionVidKey};
 use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey};
 
 use crate::{
-    infer::{region_constraints, type_variable, InferCtxtInner},
+    infer::{type_variable, InferCtxtInner},
     traits,
 };
 
@@ -25,7 +25,6 @@ pub(crate) enum UndoLog<'tcx> {
     IntUnificationTable(sv::UndoLog<ut::Delegate<ty::IntVid>>),
     FloatUnificationTable(sv::UndoLog<ut::Delegate<ty::FloatVid>>),
     EffectUnificationTable(sv::UndoLog<ut::Delegate<EffectVidKey<'tcx>>>),
-    RegionConstraintCollector(region_constraints::UndoLog<'tcx>),
     RegionUnificationTable(sv::UndoLog<ut::Delegate<RegionVidKey<'tcx>>>),
     ProjectionCache(traits::UndoLog<'tcx>),
     PushRegionObligation,
@@ -45,7 +44,6 @@ macro_rules! impl_from {
 
 // Upcast from a single kind of "undoable action" to the general enum
 impl_from! {
-    RegionConstraintCollector(region_constraints::UndoLog<'tcx>),
     TypeVariables(type_variable::UndoLog<'tcx>),
 
     TypeVariables(sv::UndoLog<ut::Delegate<type_variable::TyVidEqKey<'tcx>>>),
@@ -72,9 +70,6 @@ impl<'tcx> Rollback<UndoLog<'tcx>> for InferCtxtInner<'tcx> {
             UndoLog::IntUnificationTable(undo) => self.int_unification_storage.reverse(undo),
             UndoLog::FloatUnificationTable(undo) => self.float_unification_storage.reverse(undo),
             UndoLog::EffectUnificationTable(undo) => self.effect_unification_storage.reverse(undo),
-            UndoLog::RegionConstraintCollector(undo) => {
-                self.region_constraint_storage.as_mut().unwrap().reverse(undo)
-            }
             UndoLog::RegionUnificationTable(undo) => {
                 self.region_constraint_storage.as_mut().unwrap().unification_table.reverse(undo)
             }
@@ -170,16 +165,6 @@ impl<'tcx> InferCtxtUndoLogs<'tcx> {
         Snapshot { undo_len: self.logs.len(), _marker: PhantomData }
     }
 
-    pub(crate) fn region_constraints_in_snapshot(
-        &self,
-        s: &Snapshot<'tcx>,
-    ) -> impl Iterator<Item = &'_ region_constraints::UndoLog<'tcx>> + Clone {
-        self.logs[s.undo_len..].iter().filter_map(|log| match log {
-            UndoLog::RegionConstraintCollector(log) => Some(log),
-            _ => None,
-        })
-    }
-
     pub(crate) fn opaque_types_in_snapshot(&self, s: &Snapshot<'tcx>) -> bool {
         self.logs[s.undo_len..].iter().any(|log| matches!(log, UndoLog::OpaqueTypes(..)))
     }

compiler/rustc_trait_selection/src/traits/auto_trait.rs

@@ -477,7 +477,7 @@ impl<'tcx> AutoTraitFinder<'tcx> {
         let mut vid_map: FxHashMap<RegionTarget<'cx>, RegionDeps<'cx>> = FxHashMap::default();
         let mut finished_map = FxHashMap::default();
 
-        for constraint in regions.constraints.keys() {
+        for (constraint, _) in &regions.constraints {
             match constraint {
                 &Constraint::VarSubVar(r1, r2) => {
                     {

src/librustdoc/clean/auto_trait.rs

@@ -195,7 +195,7 @@ where
         // into a map. Each RegionTarget (either a RegionVid or a Region) maps
         // to its smaller and larger regions. Note that 'larger' regions correspond
         // to sub-regions in Rust code (e.g., in 'a: 'b, 'a is the larger region).
-        for constraint in regions.constraints.keys() {
+        for (constraint, _) in &regions.constraints {
             match *constraint {
                 Constraint::VarSubVar(r1, r2) => {
                     {

tests/ui/higher-ranked/higher-ranked-lifetime-error.stderr

@@ -4,8 +4,8 @@ error[E0308]: mismatched types
 LL |     assert_all::<_, &String>(id);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
    |
-   = note: expected reference `&String`
-              found reference `&String`
+   = note: expected trait `for<'a> <for<'a> fn(&'a String) -> &'a String {id} as FnMut<(&'a String,)>>`
+              found trait `for<'a> <for<'a> fn(&'a String) -> &'a String {id} as FnMut<(&'a String,)>>`
 
 error: aborting due to 1 previous error