Integrate caching of results. Measurements show approx 90% hit rate.

4 files changed, 98 insertions, 75 deletions

diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs
index f69eb2e17ea..ad3e1ae0242 100644
--- a/src/librustc/middle/traits/mod.rs
+++ b/src/librustc/middle/traits/mod.rs
@@ -22,6 +22,7 @@ use syntax::codemap::{Span, DUMMY_SP};
 
 pub use self::fulfill::FulfillmentContext;
 pub use self::select::SelectionContext;
+pub use self::select::SelectionCache;
 pub use self::util::supertraits;
 pub use self::util::transitive_bounds;
 pub use self::util::Supertraits;
diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs
index 5395e966887..63fbeb797c4 100644
--- a/src/librustc/middle/traits/select.rs
+++ b/src/librustc/middle/traits/select.rs
@@ -28,6 +28,8 @@ use middle::ty_fold::TypeFoldable;
 use middle::typeck::check::regionmanip;
 use middle::typeck::infer;
 use middle::typeck::infer::{InferCtxt, TypeSkolemizer};
+use std::cell::RefCell;
+use std::collections::hashmap::HashMap;
 use std::rc::Rc;
 use syntax::ast;
 use util::ppaux::Repr;
@@ -46,15 +48,15 @@ struct ObligationStack<'prev> {
     previous: Option<&'prev ObligationStack<'prev>>
 }
 
-// pub struct SelectionCache {
-//     hashmap: RefCell<HashMap<CacheKey, Candidate>>,
-// }
+pub struct SelectionCache {
+    hashmap: RefCell<HashMap<CacheKey, SelectionResult<Candidate>>>,
+}
 
-// #[deriving(Hash,Eq,PartialEq)]
-// struct CacheKey {
-//     trait_def_id: ast::DefId,
-//     skol_obligation_self_ty: ty::t,
-// }
+#[deriving(Hash,Eq,PartialEq)]
+struct CacheKey {
+    trait_def_id: ast::DefId,
+    skol_obligation_self_ty: ty::t,
+}
 
 #[deriving(PartialEq,Eq)]
 enum MatchResult<T> {
@@ -309,14 +311,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // First, check the cache.
         match self.check_candidate_cache(stack.obligation, stack.skol_obligation_self_ty) {
             Some(c) => {
-                return Ok(Some(c));
+                debug!("check_candidate_cache(obligation={}, skol_obligation_self_ty={}, \
+                       candidate={})",
+                       stack.obligation.trait_ref.def_id,
+                       stack.skol_obligation_self_ty.repr(self.tcx()),
+                       c.repr(self.tcx()));
+                return c;
             }
             None => { }
         }
 
         // If no match, compute result and insert into cache.
         let result = self.pick_candidate(stack);
-        // self.insert_candidate_cache(obligation, skol_obligation_self_ty, result.clone());
+        self.insert_candidate_cache(stack.obligation,
+                                    stack.skol_obligation_self_ty,
+                                    result.clone());
         result
     }
 
@@ -330,9 +339,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         let mut candidates = try!(self.assemble_candidates(stack));
 
-        debug!("candidate_from_obligation: {} candidates for {}",
-               candidates.len(),
-               stack.repr(self.tcx()));
+        debug!("assembled {} candidates for {}",
+               candidates.len(), stack.repr(self.tcx()));
 
         // Examine candidates to determine outcome. Ideally we will
         // have exactly one candidate that is definitively applicable.
@@ -348,55 +356,63 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 debug!("0 matches, unimpl");
                 Err(Unimplemented)
             } else {
-                debug!("candidate_from_obligation({}) -> 0 matches, ambig",
-                       stack.repr(self.tcx()));
+                debug!("0 matches, ambig");
                 Ok(None)
-            };
-        }
-
-        if candidates.len() > 1 {
+            }
+        } else if candidates.len() > 1 {
             // Ambiguity. Possibly we should report back more
             // information on the potential candidates so we can give
             // a better error message.
-            debug!("candidate_from_obligation({}) -> multiple matches, ambig",
-                   stack.repr(self.tcx()));
-
-            return Ok(None);
+            debug!("multiple matches, ambig");
+            Ok(None)
+        } else {
+            let candidate = candidates.pop().unwrap();
+            Ok(Some(candidate))
         }
+    }
 
-        let candidate = candidates.pop().unwrap();
-        self.insert_candidate_cache(stack.obligation,
-                                    stack.skol_obligation_self_ty,
-                                    candidate.clone());
-        Ok(Some(candidate))
+    fn pick_candidate_cache(&self,
+                            _obligation: &Obligation,
+                            skol_obligation_self_ty: ty::t)
+                            -> &SelectionCache
+    {
+        if
+            ty::type_has_self(skol_obligation_self_ty) ||
+            ty::type_has_params(skol_obligation_self_ty)
+        {
+            &self.param_env.selection_cache
+        } else {
+            &self.tcx().selection_cache
+        }
     }
 
     fn check_candidate_cache(&mut self,
-                             _obligation: &Obligation,
-                             _skol_obligation_self_ty: ty::t)
-                             -> Option<Candidate>
+                             obligation: &Obligation,
+                             skol_obligation_self_ty: ty::t)
+                             -> Option<SelectionResult<Candidate>>
     {
-        // let cache_key = CacheKey::new(obligation.trait_ref.def_id,
-        //                               skol_obligation_self_ty);
-        // let hashmap = self.tcx().selection_cache.hashmap.borrow();
-        // hashmap.find(&cache_key).map(|c| (*c).clone())
-        None
+        let cache = self.pick_candidate_cache(obligation, skol_obligation_self_ty);
+        let cache_key = CacheKey::new(obligation.trait_ref.def_id,
+                                      skol_obligation_self_ty);
+        let hashmap = cache.hashmap.borrow();
+        hashmap.find(&cache_key).map(|c| (*c).clone())
     }
 
     fn insert_candidate_cache(&mut self,
-                              _obligation: &Obligation,
-                              _skol_obligation_self_ty: ty::t,
-                              _candidate: Candidate)
+                              obligation: &Obligation,
+                              skol_obligation_self_ty: ty::t,
+                              candidate: SelectionResult<Candidate>)
     {
-        // FIXME -- Enable caching. I think the right place to put the cache
-        // is in the ParameterEnvironment, not the tcx, because otherwise
-        // when there are distinct where clauses in scope the cache can get
-        // confused.
-        //
-        //let cache_key = CacheKey::new(obligation.trait_ref.def_id,
-        //                              skol_obligation_self_ty);
-        //let mut hashmap = self.tcx().selection_cache.hashmap.borrow_mut();
-        //hashmap.insert(cache_key, candidate);
+        debug!("insert_candidate_cache(obligation={}, skol_obligation_self_ty={}, candidate={})",
+               obligation.trait_ref.def_id,
+               skol_obligation_self_ty.repr(self.tcx()),
+               candidate.repr(self.tcx()));
+
+        let cache = self.pick_candidate_cache(obligation, skol_obligation_self_ty);
+        let cache_key = CacheKey::new(obligation.trait_ref.def_id,
+                                      skol_obligation_self_ty);
+        let mut hashmap = cache.hashmap.borrow_mut();
+        hashmap.insert(cache_key, candidate);
     }
 
     fn assemble_candidates(&mut self,
@@ -1487,6 +1503,14 @@ impl Repr for ImplCandidate {
     }
 }
 
+impl SelectionCache {
+    pub fn new() -> SelectionCache {
+        SelectionCache {
+            hashmap: RefCell::new(HashMap::new())
+        }
+    }
+}
+
 impl<'o> ObligationStack<'o> {
     fn iter(&self) -> Option<&ObligationStack> {
         Some(self)
@@ -1515,22 +1539,14 @@ impl<'o> Repr for ObligationStack<'o> {
     }
 }
 
-// impl SelectionCache {
-//     pub fn new() -> SelectionCache {
-//         SelectionCache {
-//             hashmap: RefCell::new(HashMap::new())
-//         }
-//     }
-// }
-
-// impl CacheKey {
-//     pub fn new(trait_def_id: ast::DefId,
-//                skol_obligation_self_ty: ty::t)
-//                -> CacheKey
-//     {
-//         CacheKey {
-//             trait_def_id: trait_def_id,
-//             skol_obligation_self_ty: skol_obligation_self_ty
-//         }
-//     }
-// }
+impl CacheKey {
+    pub fn new(trait_def_id: ast::DefId,
+               skol_obligation_self_ty: ty::t)
+               -> CacheKey
+    {
+        CacheKey {
+            trait_def_id: trait_def_id,
+            skol_obligation_self_ty: skol_obligation_self_ty
+        }
+    }
+}
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
index 3014eb35e22..c54f638ab2a 100644
--- a/src/librustc/middle/ty.rs
+++ b/src/librustc/middle/ty.rs
@@ -579,6 +579,10 @@ pub struct ctxt<'tcx> {
     /// Maps def IDs of traits to information about their associated types.
     pub trait_associated_types:
         RefCell<DefIdMap<Rc<Vec<AssociatedTypeInfo>>>>,
+
+    /// Caches the results of trait selection. This cache is used
+    /// for things that do not have to do with the parameters in scope.
+    pub selection_cache: traits::SelectionCache,
 }
 
 pub enum tbox_flag {
@@ -1281,6 +1285,10 @@ pub struct ParameterEnvironment {
     /// Note: This effectively *duplicates* the `bounds` array for
     /// now.
     pub caller_obligations: VecPerParamSpace<traits::Obligation>,
+
+    /// Caches the results of trait selection. This cache is used
+    /// for things that have to do with the parameters in scope.
+    pub selection_cache: traits::SelectionCache,
 }
 
 impl ParameterEnvironment {
@@ -1524,7 +1532,8 @@ pub fn mk_ctxt<'tcx>(s: Session,
         capture_modes: capture_modes,
         associated_types: RefCell::new(DefIdMap::new()),
         trait_associated_types: RefCell::new(DefIdMap::new()),
-    }
+        selection_cache: traits::SelectionCache::new(),
+   }
 }
 
 // Type constructors
@@ -5324,7 +5333,8 @@ pub fn empty_parameter_environment() -> ParameterEnvironment {
     ty::ParameterEnvironment { free_substs: Substs::empty(),
                                bounds: VecPerParamSpace::empty(),
                                caller_obligations: VecPerParamSpace::empty(),
-                               implicit_region_bound: ty::ReEmpty }
+                               implicit_region_bound: ty::ReEmpty,
+                               selection_cache: traits::SelectionCache::new(), }
 }
 
 pub fn construct_parameter_environment(
@@ -5396,6 +5406,7 @@ pub fn construct_parameter_environment(
         bounds: bounds,
         implicit_region_bound: ty::ReScope(free_id),
         caller_obligations: obligations,
+        selection_cache: traits::SelectionCache::new(),
     };
 
     fn push_region_params(regions: &mut VecPerParamSpace<ty::Region>,
diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs
index 9009644eb0c..6ef2e7b8fca 100644
--- a/src/librustc/middle/typeck/check/mod.rs
+++ b/src/librustc/middle/typeck/check/mod.rs
@@ -368,12 +368,7 @@ fn static_inherited_fields<'a, 'tcx>(ccx: &'a CrateCtxt<'a, 'tcx>)
                                     -> Inherited<'a, 'tcx> {
     // It's kind of a kludge to manufacture a fake function context
     // and statement context, but we might as well do write the code only once
-    let param_env = ty::ParameterEnvironment {
-        free_substs: subst::Substs::empty(),
-        bounds: subst::VecPerParamSpace::empty(),
-        implicit_region_bound: ty::ReStatic,
-        caller_obligations: subst::VecPerParamSpace::empty(),
-    };
+    let param_env = ty::empty_parameter_environment();
     Inherited::new(ccx.tcx, param_env)
 }