From 04784540efe156398eee4fba535f5a24b8bd649e Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 18 Apr 2014 03:39:25 -0300 Subject: [PATCH 1/3] Remove redundant variable in LruCache::put --- src/libcollections/lru_cache.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/libcollections/lru_cache.rs b/src/libcollections/lru_cache.rs index 87b1fee1d27d9..84553782104d6 100644 --- a/src/libcollections/lru_cache.rs +++ b/src/libcollections/lru_cache.rs @@ -114,10 +114,8 @@ impl LruCache { /// Put a key-value pair into cache. pub fn put(&mut self, k: K, v: V) { - let mut key_existed = false; let (node_ptr, node_opt) = match self.map.find_mut(&KeyRef{k: &k}) { Some(node) => { - key_existed = true; node.value = Some(v); let node_ptr: *mut LruEntry = &mut **node; (node_ptr, None) @@ -128,15 +126,18 @@ impl LruCache { (node_ptr, Some(node)) } }; - if key_existed { - self.detach(node_ptr); - self.attach(node_ptr); - } else { - let keyref = unsafe { (*node_ptr).key.as_ref().unwrap() }; - self.map.swap(KeyRef{k: keyref}, node_opt.unwrap()); - self.attach(node_ptr); - if self.len() > self.capacity() { - self.remove_lru(); + match node_opt { + None => { + self.detach(node_ptr); + self.attach(node_ptr); + } + Some(node) => { + let keyref = unsafe { (*node_ptr).key.as_ref().unwrap() }; + self.map.swap(KeyRef{k: keyref}, node); + self.attach(node_ptr); + if self.len() > self.capacity() { + self.remove_lru(); + } } } } From ad4062e8af18b9631291e2c50bf0370ad2768e19 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 18 Apr 2014 07:23:36 -0300 Subject: [PATCH 2/3] Eliminate unecessary extra sigil node from LruCache. Instead of allocating both head and tail nodes for the ends of the node list, a single node can be allocated and linked circularly instead, making it act as both the head and the tail of the list at the same time. --- src/libcollections/lru_cache.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libcollections/lru_cache.rs b/src/libcollections/lru_cache.rs index 84553782104d6..ef49ada322a4b 100644 --- a/src/libcollections/lru_cache.rs +++ b/src/libcollections/lru_cache.rs @@ -59,7 +59,6 @@ pub struct LruCache { map: HashMap, ~LruEntry>, max_size: uint, head: *mut LruEntry, - tail: *mut LruEntry, } impl> Hash for KeyRef { @@ -103,11 +102,10 @@ impl LruCache { map: HashMap::new(), max_size: capacity, head: unsafe{ cast::transmute(~LruEntry::::new()) }, - tail: unsafe{ cast::transmute(~LruEntry::::new()) }, }; unsafe { - (*cache.head).next = cache.tail; - (*cache.tail).prev = cache.head; + (*cache.head).next = cache.head; + (*cache.head).prev = cache.head; } return cache; } @@ -191,7 +189,7 @@ impl LruCache { #[inline] fn remove_lru(&mut self) { if self.len() > 0 { - let lru = unsafe { (*self.tail).prev }; + let lru = unsafe { (*self.head).prev }; self.detach(lru); unsafe { match (*lru).key { @@ -269,7 +267,6 @@ impl Drop for LruCache { fn drop(&mut self) { unsafe { let _: ~LruEntry = cast::transmute(self.head); - let _: ~LruEntry = cast::transmute(self.tail); } } } From 9faef77b237baf8cf9908c799be162ab28a9aa84 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 18 Apr 2014 08:44:30 -0300 Subject: [PATCH 3/3] Eliminate the need for Options in LruEntry. LruEntry nodes previously used Option to encapsulate the key and value fields. This was used merely as a way avoid having values for the sigil node. Apart from wasting a few bytes for the discriminant, this cluttered the rest of the code, since these fields always contained Some on regular nodes as a class invariant. The Option wrapping was removed, and the values in the sigil field are initialized using mem::init, so that they don't contain any real data. --- src/libcollections/lru_cache.rs | 63 +++++++++++---------------------- 1 file changed, 21 insertions(+), 42 deletions(-) diff --git a/src/libcollections/lru_cache.rs b/src/libcollections/lru_cache.rs index ef49ada322a4b..0b7199661f8e3 100644 --- a/src/libcollections/lru_cache.rs +++ b/src/libcollections/lru_cache.rs @@ -41,6 +41,7 @@ use std::cast; use std::container::Container; use std::hash::Hash; use std::fmt; +use std::mem; use std::ptr; use HashMap; @@ -48,10 +49,10 @@ use HashMap; struct KeyRef { k: *K } struct LruEntry { - key: Option, - value: Option, next: *mut LruEntry, prev: *mut LruEntry, + key: K, + value: V, } /// An LRU Cache. @@ -76,19 +77,10 @@ impl Eq for KeyRef { impl TotalEq for KeyRef {} impl LruEntry { - fn new() -> LruEntry { + fn new(k: K, v: V) -> LruEntry { LruEntry { - key: None, - value: None, - next: ptr::mut_null(), - prev: ptr::mut_null(), - } - } - - fn with_key_value(k: K, v: V) -> LruEntry { - LruEntry { - key: Some(k), - value: Some(v), + key: k, + value: v, next: ptr::mut_null(), prev: ptr::mut_null(), } @@ -101,7 +93,7 @@ impl LruCache { let cache = LruCache { map: HashMap::new(), max_size: capacity, - head: unsafe{ cast::transmute(~LruEntry::::new()) }, + head: unsafe{ cast::transmute(~mem::uninit::>()) }, }; unsafe { (*cache.head).next = cache.head; @@ -114,23 +106,24 @@ impl LruCache { pub fn put(&mut self, k: K, v: V) { let (node_ptr, node_opt) = match self.map.find_mut(&KeyRef{k: &k}) { Some(node) => { - node.value = Some(v); + node.value = v; let node_ptr: *mut LruEntry = &mut **node; (node_ptr, None) } None => { - let mut node = ~LruEntry::with_key_value(k, v); + let mut node = ~LruEntry::new(k, v); let node_ptr: *mut LruEntry = &mut *node; (node_ptr, Some(node)) } }; match node_opt { None => { + // Existing node, just update LRU position self.detach(node_ptr); self.attach(node_ptr); } Some(node) => { - let keyref = unsafe { (*node_ptr).key.as_ref().unwrap() }; + let keyref = unsafe { &(*node_ptr).key }; self.map.swap(KeyRef{k: keyref}, node); self.attach(node_ptr); if self.len() > self.capacity() { @@ -146,12 +139,7 @@ impl LruCache { None => (None, None), Some(node) => { let node_ptr: *mut LruEntry = &mut **node; - unsafe { - match (*node_ptr).value { - None => (None, None), - Some(ref value) => (Some(value), Some(node_ptr)) - } - } + (Some(unsafe { &(*node_ptr).value }), Some(node_ptr)) } }; match node_ptr_opt { @@ -168,7 +156,7 @@ impl LruCache { pub fn pop(&mut self, k: &K) -> Option { match self.map.pop(&KeyRef{k: k}) { None => None, - Some(lru_entry) => lru_entry.value + Some(lru_entry) => Some(lru_entry.value) } } @@ -191,12 +179,7 @@ impl LruCache { if self.len() > 0 { let lru = unsafe { (*self.head).prev }; self.detach(lru); - unsafe { - match (*lru).key { - None => (), - Some(ref k) => { self.map.pop(&KeyRef{k: k}); } - } - } + self.map.pop(&KeyRef{k: unsafe { &(*lru).key }}); } } @@ -229,19 +212,11 @@ impl fmt::Show for LruCache { if i > 0 { try!(write!(f.buf, ", ")) } unsafe { cur = (*cur).next; - match (*cur).key { - // should never print nil - None => try!(write!(f.buf, "nil")), - Some(ref k) => try!(write!(f.buf, "{}", *k)), - } + try!(write!(f.buf, "{}", (*cur).key)); } try!(write!(f.buf, ": ")); unsafe { - match (*cur).value { - // should never print nil - None => try!(write!(f.buf, "nil")), - Some(ref value) => try!(write!(f.buf, "{}", *value)), - } + try!(write!(f.buf, "{}", (*cur).value)); } } write!(f.buf, r"\}") @@ -266,7 +241,11 @@ impl Mutable for LruCache { impl Drop for LruCache { fn drop(&mut self) { unsafe { - let _: ~LruEntry = cast::transmute(self.head); + let node: ~LruEntry = cast::transmute(self.head); + // Prevent compiler from trying to drop the un-initialized field in the sigil node. + let ~LruEntry { key: k, value: v, .. } = node; + cast::forget(k); + cast::forget(v); } } }