Merge pull request #525 from sfowler/font-cache

Add an LRU cache and use it for fonts and font groups

2 files changed, 147 insertions, 30 deletions

diff --git a/src/components/gfx/font_context.rs b/src/components/gfx/font_context.rs
index 310b75944e5..4ac07bed4fd 100644
--- a/src/components/gfx/font_context.rs
+++ b/src/components/gfx/font_context.rs
@@ -2,11 +2,12 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use font::{Font, FontDescriptor, FontGroup, FontStyle, SelectorPlatformIdentifier};
+use font::{Font, FontDescriptor, FontGroup, FontHandleMethods, FontStyle,
+           SelectorPlatformIdentifier};
 use font::{SpecifiedFontStyle, UsedFontStyle};
 use font_list::FontList;
 use servo_util::cache::Cache;
-use servo_util::cache::MonoCache;
+use servo_util::cache::LRUCache;
 use servo_util::time::ProfilerChan;
 
 use platform::font::FontHandle;
@@ -35,8 +36,9 @@ pub trait FontContextHandleMethods {
 
 #[allow(non_implicitly_copyable_typarams)]
 pub struct FontContext {
-    instance_cache: MonoCache<FontDescriptor, @mut Font>,
+    instance_cache: LRUCache<FontDescriptor, @mut Font>,
     font_list: Option<FontList>, // only needed by layout
+    group_cache: LRUCache<SpecifiedFontStyle, @FontGroup>,
     handle: FontContextHandle,
     backend: BackendType,
     generic_fonts: HashMap<~str,~str>,
@@ -63,10 +65,9 @@ pub impl<'self> FontContext {
         generic_fonts.insert(~"monospace", ~"Menlo");
 
         FontContext { 
-            // TODO(Rust #3902): remove extraneous type parameters once they are inferred correctly.
-            instance_cache:
-                Cache::new::<FontDescriptor,@mut Font,MonoCache<FontDescriptor,@mut Font>>(10),
+            instance_cache: LRUCache::new(10),
             font_list: font_list,
+            group_cache: LRUCache::new(10),
             handle: handle,
             backend: backend,
             generic_fonts: generic_fonts,
@@ -78,15 +79,29 @@ pub impl<'self> FontContext {
         self.font_list.get_ref()
     }
 
-    fn get_resolved_font_for_style(@mut self, style: &SpecifiedFontStyle) -> @FontGroup {
-        // TODO(Issue #178, E): implement a cache of FontGroup instances.
-        self.create_font_group(style)
+    fn get_resolved_font_for_style(&mut self, style: &SpecifiedFontStyle) -> @FontGroup {
+        match self.group_cache.find(style) {
+            Some(fg) => {
+                debug!("font group cache hit");
+                fg
+            },
+            None => {
+                debug!("font group cache miss");
+                let fg = self.create_font_group(style);
+                self.group_cache.insert(style, fg);
+                fg
+            }
+        }
     }
 
     fn get_font_by_descriptor(&mut self, desc: &FontDescriptor) -> Result<@mut Font, ()> {
         match self.instance_cache.find(desc) {
-            Some(f) => Ok(f),
+            Some(f) => {
+                debug!("font cache hit");
+                Ok(f)
+            },
             None => { 
+                debug!("font cache miss");
                 let result = self.create_font_instance(desc);
                 match result {
                     Ok(font) => {
@@ -108,27 +123,34 @@ pub impl<'self> FontContext {
         }
     }
 
-    // TODO:(Issue #196): cache font groups on the font context.
-    priv fn create_font_group(@mut self, style: &SpecifiedFontStyle) -> @FontGroup {
+    priv fn create_font_group(&mut self, style: &SpecifiedFontStyle) -> @FontGroup {
         let mut fonts = ~[];
 
         debug!("(create font group) --- starting ---");
 
-        let list = self.get_font_list();
-
         // TODO(Issue #193): make iteration over 'font-family' more robust.
         for str::each_split_char(style.families, ',') |family| {
             let family_name = str::trim(family);
             let transformed_family_name = self.transform_family(family_name);
             debug!("(create font group) transformed family is `%s`", transformed_family_name);
 
-            let result = list.find_font_in_family(transformed_family_name, style);
+            let result = match self.font_list {
+                Some(ref fl) => {
+                    fl.find_font_in_family(transformed_family_name, style)
+                },
+                None => None,
+            };
+
             let mut found = false;
             for result.each |font_entry| {
                 found = true;
-                // TODO(Issue #203): route this instantion through FontContext's Font instance cache.
-                let instance = Font::new_from_existing_handle(self, &font_entry.handle, style, self.backend,
-                                                              self.profiler_chan.clone());
+
+                let font_id =
+                  SelectorPlatformIdentifier(font_entry.handle.face_identifier());
+                let font_desc = FontDescriptor::new(copy *style, font_id);
+
+                let instance = self.get_font_by_descriptor(&font_desc);
+
                 do result::iter(&instance) |font: &@mut Font| { fonts.push(*font); }
             };
 
@@ -140,13 +162,20 @@ pub impl<'self> FontContext {
         let last_resort = FontList::get_last_resort_font_families();
 
         for last_resort.each |family| {
-            let result = list.find_font_in_family(*family,style);
+            let result = match self.font_list {
+                Some(ref fl) => {
+                    fl.find_font_in_family(*family, style)
+                },
+                None => None,
+            };
+
             for result.each |font_entry| {
-                let instance = Font::new_from_existing_handle(self,
-                                                              &font_entry.handle,
-                                                              style,
-                                                              self.backend,
-                                                              self.profiler_chan.clone());
+                let font_id =
+                  SelectorPlatformIdentifier(font_entry.handle.face_identifier());
+                let font_desc = FontDescriptor::new(copy *style, font_id);
+
+                let instance = self.get_font_by_descriptor(&font_desc);
+
                 do result::iter(&instance) |font: &@mut Font| {
                     fonts.push(*font);
                 }
diff --git a/src/components/util/cache.rs b/src/components/util/cache.rs
index d6c46065ee7..eb2be31afcf 100644
--- a/src/components/util/cache.rs
+++ b/src/components/util/cache.rs
@@ -3,9 +3,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 pub trait Cache<K: Copy + Eq, V: Copy> {
-    fn new(size: uint) -> Self;
     fn insert(&mut self, key: &K, value: V);
-    fn find(&self, key: &K) -> Option<V>;
+    fn find(&mut self, key: &K) -> Option<V>;
     fn find_or_create(&mut self, key: &K, blk: &fn(&K) -> V) -> V;
     fn evict_all(&mut self);
 }
@@ -14,16 +13,18 @@ pub struct MonoCache<K, V> {
     entry: Option<(K,V)>,
 }
 
-impl<K: Copy + Eq, V: Copy> Cache<K,V> for MonoCache<K,V> {
+pub impl<K: Copy + Eq, V: Copy> MonoCache<K,V> {
     fn new(_size: uint) -> MonoCache<K,V> {
         MonoCache { entry: None }
     }
+}
 
+impl<K: Copy + Eq, V: Copy> Cache<K,V> for MonoCache<K,V> {
     fn insert(&mut self, key: &K, value: V) {
         self.entry = Some((copy *key, value));
     }
 
-    fn find(&self, key: &K) -> Option<V> {
+    fn find(&mut self, key: &K) -> Option<V> {
         match self.entry {
             None => None,
             Some((ref k,v)) => if *k == *key { Some(v) } else { None }
@@ -47,8 +48,7 @@ impl<K: Copy + Eq, V: Copy> Cache<K,V> for MonoCache<K,V> {
 
 #[test]
 fn test_monocache() {
-    // TODO: this is hideous because of Rust Issue #3902
-    let cache = cache::new::<uint, @str, MonoCache<uint, @str>>(10);
+    let cache = MonoCache::new(10);
     let one = @"one";
     let two = @"two";
     cache.insert(&1, one);
@@ -59,3 +59,91 @@ fn test_monocache() {
     assert!(cache.find(&2).is_some());
     assert!(cache.find(&1).is_none());
 }
+
+pub struct LRUCache<K, V> {
+    entries: ~[(K, V)],
+    cache_size: uint,
+}
+
+pub impl<K: Copy + Eq, V: Copy> LRUCache<K,V> {
+    fn new(size: uint) -> LRUCache<K, V> {
+        LRUCache {
+          entries: ~[],
+          cache_size: size,
+        }
+    }
+
+    fn touch(&mut self, pos: uint) -> V {
+        let (key, val) = copy self.entries[pos];
+        if pos != self.cache_size {
+            self.entries.remove(pos);
+            self.entries.push((key, val));
+        }
+        val
+    }
+}
+
+impl<K: Copy + Eq, V: Copy> Cache<K,V> for LRUCache<K,V> {
+    fn insert(&mut self, key: &K, val: V) {
+        if self.entries.len() == self.cache_size {
+            self.entries.remove(0);
+        }
+        self.entries.push((copy *key, val));
+    }
+
+    fn find(&mut self, key: &K) -> Option<V> {
+        match self.entries.position(|&(k, _)| k == *key) {
+            Some(pos) => Some(self.touch(pos)),
+            None      => None,
+        }
+    }
+
+    fn find_or_create(&mut self, key: &K, blk: &fn(&K) -> V) -> V {
+        match self.entries.position(|&(k, _)| k == *key) {
+            Some(pos) => self.touch(pos),
+            None => {
+              let val = blk(key);
+              self.insert(key, val);
+              val
+            }
+        }
+    }
+
+    fn evict_all(&mut self) {
+        self.entries.clear();
+    }
+}
+
+#[test]
+fn test_lru_cache() {
+    let one = @"one";
+    let two = @"two";
+    let three = @"three";
+    let four = @"four";
+
+    // Test normal insertion.
+    let cache = LRUCache::new(2); // (_, _) (cache is empty)
+    cache.insert(&1, one);    // (1, _)
+    cache.insert(&2, two);    // (1, 2)
+    cache.insert(&3, three);  // (2, 3)
+
+    assert!(cache.find(&1).is_none());  // (2, 3) (no change)
+    assert!(cache.find(&3).is_some());  // (2, 3)
+    assert!(cache.find(&2).is_some());  // (3, 2)
+
+    // Test that LRU works (this insertion should replace 3, not 2).
+    cache.insert(&4, four); // (2, 4)
+
+    assert!(cache.find(&1).is_none());  // (2, 4) (no change)
+    assert!(cache.find(&2).is_some());  // (4, 2)
+    assert!(cache.find(&3).is_none());  // (4, 2) (no change)
+    assert!(cache.find(&4).is_some());  // (2, 4) (no change)
+
+    // Test find_or_create.
+    do cache.find_or_create(&1) |_| { one } // (4, 1)
+
+    assert!(cache.find(&1).is_some()); // (4, 1) (no change)
+    assert!(cache.find(&2).is_none()); // (4, 1) (no change)
+    assert!(cache.find(&3).is_none()); // (4, 1) (no change)
+    assert!(cache.find(&4).is_some()); // (1, 4)
+}