From d569cdc2341ca7ad70f3b3bf83ae7377a3d7575f Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 30 Jan 2019 23:52:00 +0100 Subject: [PATCH] perf(check): Re-use the allocations for variables --- base/src/fixed.rs | 10 +++++- base/src/types/mod.rs | 10 ++++++ benches/check.rs | 4 ++- check/src/kindcheck.rs | 8 +++++ check/src/substitution.rs | 67 +++++++++++++++++++++++++-------------- check/src/typecheck.rs | 1 + check/src/unify.rs | 9 ++++++ check/src/unify_type.rs | 8 +++++ 8 files changed, 91 insertions(+), 26 deletions(-) diff --git a/base/src/fixed.rs b/base/src/fixed.rs index 335067d1de..74339e3a9d 100644 --- a/base/src/fixed.rs +++ b/base/src/fixed.rs @@ -54,7 +54,6 @@ impl FixedMap { } pub fn clear(&mut self) { - error!("Clear"); self.map.borrow_mut().clear(); } @@ -210,6 +209,11 @@ impl FixedVecMap { self.map.get_mut().retain(|i, _| i < index); self.values.truncate(index); } + + pub fn drain<'a>(&'a mut self) -> impl Iterator + 'a { + self.map.get_mut().clear(); + self.values.drain() + } } impl Index for FixedVecMap { @@ -368,6 +372,10 @@ impl Buffer { } } + fn drain<'a>(&'a mut self) -> impl Iterator + 'a { + self.values.get_mut().drain(..).flat_map(|vec| vec) + } + fn pop(&mut self) -> Option { let values = self.values.get_mut(); let out = values.last_mut().and_then(|vec| vec.pop()); diff --git a/base/src/types/mod.rs b/base/src/types/mod.rs index e3a0542064..61e15a9d2d 100644 --- a/base/src/types/mod.rs +++ b/base/src/types/mod.rs @@ -1224,6 +1224,7 @@ where } } +#[derive(Clone)] struct ArcTypeInner { typ: Type>, flags: Flags, @@ -1929,6 +1930,15 @@ where } impl ArcType { + pub fn set(into: &mut Self, typ: Type) + where + Id: Clone, + { + let into = Arc::make_mut(&mut into.typ); + into.flags = Flags::from_type(&typ); + into.typ = typ; + } + /// Returns the lowest level which this type contains. The level informs from where type /// variables where created. pub fn level(&self) -> u32 { diff --git a/benches/check.rs b/benches/check.rs index f3bcf045b3..df56393d98 100644 --- a/benches/check.rs +++ b/benches/check.rs @@ -110,7 +110,9 @@ fn typecheck_benchmark(c: &mut Criterion) { b.iter(|| { let vm = new_vm(); let mut compiler = Compiler::new(); - compiler.load_file(&vm, "examples/lisp/lisp.glu").unwrap() + compiler + .load_file(&vm, "examples/lisp/lisp.glu") + .unwrap_or_else(|err| panic!("{}", err)) }) }); } diff --git a/check/src/kindcheck.rs b/check/src/kindcheck.rs index 44e3947a1c..7f6c9e2071 100644 --- a/check/src/kindcheck.rs +++ b/check/src/kindcheck.rs @@ -505,6 +505,14 @@ impl Substitutable for ArcKind { Kind::variable(x) } + fn into_variable(&mut self, x: Self::Variable) { + *ArcKind::make_mut(self) = Kind::Variable(x); + } + + fn is_unique(self_: &Self) -> bool { + ArcKind::strong_count(self_) == 1 + } + fn get_var(&self) -> Option<&u32> { match **self { Kind::Variable(ref var) => Some(var), diff --git a/check/src/substitution.rs b/check/src/substitution.rs index 1b2d0993f7..a371e98da0 100644 --- a/check/src/substitution.rs +++ b/check/src/substitution.rs @@ -6,7 +6,7 @@ use crate::base::{ fixed::{FixedVec, FixedVecMap}, kind::ArcKind, symbol::Symbol, - types::{self, ArcType, Flags, FlagsVisitor, Skolem, Type, TypeContext, TypeExt, Walker}, + types::{self, ArcType, Flags, FlagsVisitor, Skolem, Type, TypeContext, Walker}, }; use crate::typ::RcType; @@ -45,6 +45,7 @@ where types: FixedVecMap, factory: T::Factory, interner: T::Interner, + variable_cache: RefCell>, } impl TypeContext for Substitution @@ -120,6 +121,10 @@ pub trait Substitutable: Sized { /// Constructs a new object from its variable type fn from_variable(subs: &Substitution, x: Self::Variable) -> Self; + fn into_variable(&mut self, x: Self::Variable); + + fn is_unique(self_: &Self) -> bool; + /// Retrieves the variable if `self` is a variable otherwise returns `None` fn get_var(&self) -> Option<&Self::Variable>; @@ -263,6 +268,7 @@ where types: FixedVecMap::new(), factory: factory, interner, + variable_cache: Default::default(), } } @@ -288,12 +294,37 @@ where self.types.insert(var as usize, t.into()); } + /// Assumes that no variables unified with anything (but variables < level may exist) + pub fn clear_from(&mut self, level: u32) { + self.union = RefCell::new(QuickFindUf::new(0)); + let mut u = self.union.borrow_mut(); + for _ in 0..level { + u.insert(UnionByLevel { + ..UnionByLevel::default() + }); + } + + let mut variable_cache = self.variable_cache.borrow_mut(); + // Since no types should be unified with anything we can remove all of this and reuse the + // unique values + variable_cache.extend(self.types.drain().filter(T::is_unique)); + while self.variables.len() > level as usize { + variable_cache.push(self.variables.pop().unwrap()); + } + } + /// Creates a new variable pub fn new_var(&self) -> T where T: Clone, { - self.new_var_fn(|var| T::from_variable(self, self.factory.new(var))) + self.new_var_fn(|var| match self.variable_cache.borrow_mut().pop() { + Some(mut typ) => { + T::into_variable(&mut typ, self.factory.new(var)); + typ + } + None => T::from_variable(self, self.factory.new(var)), + }) } pub fn new_var_fn(&self, f: F) -> T @@ -448,7 +479,16 @@ impl Substitution { impl Substitution { pub fn new_skolem(&self, name: Symbol, kind: ArcKind) -> RcType { - self.new_var_fn(|id| (&*self).skolem(Skolem { name, id, kind })) + self.new_var_fn(|id| { + let skolem = Skolem { name, id, kind }; + match self.variable_cache.borrow_mut().pop() { + Some(mut typ) => { + RcType::set(&mut typ, Type::Skolem(skolem)); + typ + } + None => (&*self).skolem(skolem), + } + }) } pub fn zonk(&self, typ: &RcType) -> RcType { @@ -468,25 +508,4 @@ impl Substitution { pub fn bind_arc(&self, typ: &RcType) -> ArcType { typ.clone() } - - /// Assumes that all variables < `level` are not unified with anything - pub fn clear_from(&mut self, level: u32) { - self.union = RefCell::new(QuickFindUf::new(0)); - let mut u = self.union.borrow_mut(); - for _ in 0..level { - u.insert(UnionByLevel { - ..UnionByLevel::default() - }); - } - self.types.truncate(level as usize); - for t in self.variables.iter().skip(level as usize) { - assert_eq!( - RcType::strong_count(t), - 1, - "Variable {} is not unbound at this stage", - t - ); - } - self.variables.truncate(level as usize); - } } diff --git a/check/src/typecheck.rs b/check/src/typecheck.rs index 015b263cf0..3fa50ae758 100644 --- a/check/src/typecheck.rs +++ b/check/src/typecheck.rs @@ -2153,6 +2153,7 @@ impl<'a> Typecheck<'a> { self.generalize_type_errors(&mut errors); self.errors = errors; + // Clear any location which could have skolems or variables left in them self.named_variables.clear(); self.implicit_resolver.implicit_vars.clear(); diff --git a/check/src/unify.rs b/check/src/unify.rs index 6438e98f32..0c3acd5888 100644 --- a/check/src/unify.rs +++ b/check/src/unify.rs @@ -282,6 +282,15 @@ mod test { fn from_variable(_: &Substitution, var: u32) -> TType { TType(Box::new(Type::Variable(var))) } + + fn into_variable(&mut self, x: Self::Variable) { + *self.0 = Type::Variable(x); + } + + fn is_unique(self_: &Self) -> bool { + true + } + fn get_var(&self) -> Option<&u32> { match *self.0 { Type::Variable(ref var) => Some(var), diff --git a/check/src/unify_type.rs b/check/src/unify_type.rs index a21c08dcfc..ab4d82d8d5 100644 --- a/check/src/unify_type.rs +++ b/check/src/unify_type.rs @@ -246,6 +246,14 @@ impl Substitutable for RcType { subs.variable(var) } + fn into_variable(&mut self, x: Self::Variable) { + Self::set(self, Type::Variable(x)) + } + + fn is_unique(self_: &Self) -> bool { + RcType::strong_count(self_) == 1 + } + fn get_var(&self) -> Option<&TypeVariable> { match **self { Type::Variable(ref var) => Some(var),