perf: Use VecMap for type variable -> type lookup

base/Cargo.toml

@@ -26,6 +26,7 @@ codespan = "0.2"
 codespan-reporting = "0.2"
 either = "1"
 stable_deref_trait = "1"
+vec_map = "0.8"
 
 serde = { version = "1.0.0", features = ["rc"], optional = true }
 serde_state = { version = "0.4.0", features = ["rc"], optional = true }

base/src/fixed.rs

@@ -13,6 +13,8 @@ use std::{
 };
 
 use crate::fnv::FnvMap;
+use vec_map::VecMap;
+
 // NOTE: transmute is used to circumvent the borrow checker in this module
 // This is safe since the containers hold boxed values meaning allocating larger
 // storage does not invalidate the references that are handed out and because values
@@ -114,6 +116,88 @@ where
     }
 }
 
+// A mapping between K and V where once a value has been inserted it cannot be changed
+// Through this and the fact the all values are stored as pointers it is possible to safely
+// insert new values without invalidating pointers retrieved from it
+pub struct FixedVecMap<V> {
+    // Use u16 to leave space for the `Option` tag in `VecMap`
+    map: RefCell<VecMap<(u16, u32)>>,
+    values: Buffer<V>,
+}
+
+impl<V> Default for FixedVecMap<V> {
+    fn default() -> FixedVecMap<V> {
+        FixedVecMap::new()
+    }
+}
+
+impl<V: fmt::Debug> fmt::Debug for FixedVecMap<V> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.map.borrow().fmt(f)
+    }
+}
+
+impl<V> FixedVecMap<V> {
+    pub fn new() -> FixedVecMap<V> {
+        FixedVecMap {
+            map: Default::default(),
+            values: Default::default(),
+        }
+    }
+
+    pub fn clear(&mut self) {
+        self.map.borrow_mut().clear();
+    }
+
+    pub fn len(&self) -> usize {
+        self.map.borrow().len()
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+}
+
+impl<V> FixedVecMap<V> {
+    pub fn insert(&mut self, key: usize, value: V) -> Option<V> {
+        use vec_map::Entry;
+        match self.map.get_mut().entry(key) {
+            Entry::Occupied(entry) => Some(mem::replace(&mut self.values[*entry.get()], value)),
+            Entry::Vacant(entry) => {
+                let (i, j) = self.values.push(value);
+                entry.insert((i as u16, j));
+                None
+            }
+        }
+    }
+
+    pub fn try_insert(&self, key: usize, value: V) -> Result<(), (usize, V)> {
+        if self.get(key).is_some() {
+            Err((key, value))
+        } else {
+            let (i, j) = self.values.push(value);
+            self.map.borrow_mut().insert(key, (i as u16, j));
+            Ok(())
+        }
+    }
+
+    pub fn get(&self, k: usize) -> Option<&V> {
+        self.map.borrow().get(k).map(|key| &self.values[*key])
+    }
+
+    pub fn get_mut(&mut self, k: usize) -> Option<&mut V> {
+        let values = &mut self.values;
+        self.map.get_mut().get(k).map(move |&key| &mut values[key])
+    }
+}
+
+impl<V> Index<usize> for FixedVecMap<V> {
+    type Output = V;
+    fn index(&self, index: usize) -> &Self::Output {
+        self.get(index).expect("Index out of bounds")
+    }
+}
+
 #[derive(Debug)]
 pub struct FixedVec<T> {
     vec: RefCell<Vec<(u32, u32)>>,
@@ -241,3 +325,16 @@ impl<T> IndexMut<(u32, u32)> for Buffer<T> {
         &mut self.values.get_mut()[i as usize][j as usize]
     }
 }
+
+impl<T> Index<(u16, u32)> for Buffer<T> {
+    type Output = T;
+    fn index(&self, (i, j): (u16, u32)) -> &T {
+        unsafe { forget_lifetime(&self.values.borrow()[i as usize][j as usize]) }
+    }
+}
+
+impl<T> IndexMut<(u16, u32)> for Buffer<T> {
+    fn index_mut(&mut self, (i, j): (u16, u32)) -> &mut T {
+        &mut self.values.get_mut()[i as usize][j as usize]
+    }
+}

check/src/substitution.rs

@@ -3,7 +3,7 @@ use std::{cell::RefCell, default::Default, fmt};
 use union_find::{QuickFindUf, Union, UnionByRank, UnionFind, UnionResult};
 
 use crate::base::{
-    fixed::{FixedMap, FixedVec},
+    fixed::{FixedVec, FixedVecMap},
     kind::ArcKind,
     symbol::Symbol,
     types::{self, ArcType, InternerVisitor, Skolem, Type, TypeInterner, Walker},
@@ -40,9 +40,9 @@ where
     /// which needs to always be able to return a `&T` reference
     variables: FixedVec<T>,
     /// For variables which have been infered to have a real type (not a variable) their types are
-    /// stored here. As the type stored will never changed we use a `FixedMap` lets `real` return
+    /// stored here. As the type stored will never changed we use a `FixedVecMap` lets `real` return
     /// `&T` from this map safely.
-    types: FixedMap<u32, T>,
+    types: FixedVecMap<T>,
     factory: T::Factory,
     interner: T::Interner,
 }
@@ -264,7 +264,7 @@ where
         Substitution {
             union: RefCell::new(QuickFindUf::new(0)),
             variables: FixedVec::new(),
-            types: FixedMap::new(),
+            types: FixedVecMap::new(),
             factory: factory,
             interner,
         }
@@ -285,7 +285,7 @@ where
                 "Tried to insert variable which is not allowed as that would cause memory \
                  unsafety"
             ),
-            None => match self.types.try_insert(var, t.into()) {
+            None => match self.types.try_insert(var as usize, t.into()) {
                 Ok(()) => (),
                 Err(_) => ice!("Expected variable to not have a type associated with it"),
             },
@@ -294,7 +294,7 @@ where
 
     pub fn replace(&mut self, var: u32, t: T) {
         debug_assert!(t.get_id() != Some(var));
-        self.types.insert(var, t.into());
+        self.types.insert(var as usize, t.into());
     }
 
     /// Creates a new variable
@@ -344,9 +344,9 @@ where
         if var as usize >= union.size() {
             return None;
         }
-        let index = union.find(var as usize) as u32;
-        self.types.get(&index).or_else(|| {
-            if var == index {
+        let index = union.find(var as usize);
+        self.types.get(index).or_else(|| {
+            if var == index as u32 {
                 None
             } else {
                 Some(&self.variables[index as usize])

check/src/typecheck/generalize.rs

@@ -93,10 +93,8 @@ impl<'a, 'b> TypeGeneralizer<'a, 'b> {
             type Ident = Symbol;
 
             fn visit_expr(&mut self, e: &'d mut SpannedExpr<Self::Ident>) {
-                self.generalizer.tc.type_variables.enter_scope();
                 self.generalizer.span = e.span;
                 ast::walk_mut_expr(self, e);
-                self.generalizer.tc.type_variables.exit_scope();
             }
 
             fn visit_spanned_typed_ident(&mut self, id: &mut SpannedIdent<Symbol>) {
@@ -213,9 +211,9 @@ impl<'a, 'b> TypeGeneralizer<'a, 'b> {
             }
 
             _ => {
-                self.type_variables.enter_scope();
                 // Ensure that the forall's variables don't look unbound
                 if let Type::Forall(ref params, _) = **typ {
+                    self.type_variables.enter_scope();
                     let mut type_cache = &self.tc.subs;
                     self.tc.type_variables.extend(
                         params
@@ -233,7 +231,9 @@ impl<'a, 'b> TypeGeneralizer<'a, 'b> {
                 .map(|t| unroll_typ(self.tc, &t).unwrap_or(t))
                 .or_else(|| replacement.clone());
 
-                self.type_variables.exit_scope();
+                if let Type::Forall(..) = **typ {
+                    self.type_variables.exit_scope();
+                }
 
                 new_type
             }