refactor(headers): changed from rwlock to mucell underneath

seanmonstar · seanmonstar · commit d71d6549f7cd · 2014-11-29T22:06:48.000-08:00
diff --git a/Cargo.toml b/Cargo.toml
@@ -27,3 +27,6 @@ git = "https://github.com/alexcrichton/cookie-rs"
 
 [dependencies.time]
 git = "https://github.com/rust-lang/time"
+
+[dependencies.mucell]
+git = "https://github.com/chris-morgan/mucell"
diff --git a/src/header/common/mod.rs b/src/header/common/mod.rs
@@ -26,33 +26,24 @@ macro_rules! bench_header(
         #[cfg(test)]
         mod $name {
             use test::Bencher;
-            use std::fmt::{mod, Show};
-
             use super::*;
 
-            use header::{Header, HeaderFormat};
-
-            struct HeaderFormatter($ty);
-
-            impl Show for HeaderFormatter {
-                fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-                    self.0.fmt_header(f)
-                }
-            }
+            use header::{Header, HeaderFormatter};
 
             #[bench]
             fn bench_parse(b: &mut Bencher) {
                 let val = $value;
                 b.iter(|| {
-                    let _: $ty= Header::parse_header(val[]).unwrap();
+                    let _: $ty = Header::parse_header(val[]).unwrap();
                 });
             }
 
             #[bench]
             fn bench_format(b: &mut Bencher) {
-                let val = HeaderFormatter(Header::parse_header($value[]).unwrap());
+                let val: $ty = Header::parse_header($value[]).unwrap();
+                let fmt = HeaderFormatter(&val);
                 b.iter(|| {
-                    format!("{}", val);
+                    format!("{}", fmt);
                 });
             }
         }
diff --git a/src/header/mod.rs b/src/header/mod.rs
@@ -13,9 +13,9 @@ use std::raw::TraitObject;
 use std::str::SendStr;
 use std::collections::HashMap;
 use std::collections::hash_map::{Entries, Occupied, Vacant};
-use std::sync::RWLock;
 use std::{hash, mem};
 
+use mucell::MuCell;
 use uany::{UncheckedAnyDowncast, UncheckedAnyMutDowncast};
 
 use http::{mod, LineEnding};
@@ -96,7 +96,7 @@ fn header_name<T: Header>() -> &'static str {
 /// A map of header fields on requests and responses.
 #[deriving(Clone)]
 pub struct Headers {
-    data: HashMap<CaseInsensitive, RWLock<Item>>
+    data: HashMap<CaseInsensitive, MuCell<Item>>
 }
 
 impl Headers {
@@ -116,12 +116,12 @@ impl Headers {
                 Some((name, value)) => {
                     debug!("raw header: {}={}", name, value[].to_ascii());
                     let name = CaseInsensitive(Owned(name));
-                    let item = match headers.data.entry(name) {
-                        Vacant(entry) => entry.set(RWLock::new(Item::raw(vec![]))),
+                    let mut item = match headers.data.entry(name) {
+                        Vacant(entry) => entry.set(MuCell::new(Item::raw(vec![]))),
                         Occupied(entry) => entry.into_mut()
                     };
 
-                    match &mut item.write().raw {
+                    match &mut item.borrow_mut().raw {
                         &Some(ref mut raw) => raw.push(value),
                         // Unreachable
                         _ => {}
@@ -138,7 +138,7 @@ impl Headers {
     /// The field is determined by the type of the value being set.
     pub fn set<H: Header + HeaderFormat>(&mut self, value: H) {
         self.data.insert(CaseInsensitive(Borrowed(header_name::<H>())),
-                         RWLock::new(Item::typed(box value as Box<HeaderFormat + Send + Sync>)));
+                         MuCell::new(Item::typed(box value as Box<HeaderFormat + Send + Sync>)));
     }
 
     /// Access the raw value of a header.
@@ -157,15 +157,19 @@ impl Headers {
             // FIXME(reem): Find a better way to do this lookup without find_equiv.
             .get(&CaseInsensitive(Borrowed(unsafe { mem::transmute::<&str, &str>(name) })))
             .and_then(|item| {
-                let lock = item.read();
-                if let Some(ref raw) = lock.raw {
+                if let Some(ref raw) = item.borrow().raw {
                     return unsafe { mem::transmute(Some(raw[])) };
                 }
 
-                let mut lock = item.write();
-                let raw = vec![lock.typed.as_ref().unwrap().to_string().into_bytes()];
-                lock.raw = Some(raw);
-                unsafe { mem::transmute(Some(lock.raw.as_ref().unwrap()[])) }
+                let worked = item.try_mutate(|item| {
+                    let raw = vec![item.typed.as_ref().unwrap().to_string().into_bytes()];
+                    item.raw = Some(raw);
+                });
+                debug_assert!(worked, "item.try_mutate should return true");
+
+                let item = item.borrow();
+                let raw = item.raw.as_ref().unwrap();
+                unsafe { mem::transmute(Some(raw[])) }
             })
     }
 
@@ -179,29 +183,31 @@ impl Headers {
     /// headers.set_raw("content-length", vec!["5".as_bytes().to_vec()]);
     /// ```
     pub fn set_raw<K: IntoCow<'static, String, str>>(&mut self, name: K, value: Vec<Vec<u8>>) {
-        self.data.insert(CaseInsensitive(name.into_cow()), RWLock::new(Item::raw(value)));
+        self.data.insert(CaseInsensitive(name.into_cow()), MuCell::new(Item::raw(value)));
     }
 
     /// Get a reference to the header field's value, if it exists.
     pub fn get<H: Header + HeaderFormat>(&self) -> Option<&H> {
         self.get_or_parse::<H>().map(|item| {
             unsafe {
-                mem::transmute::<&H, &H>(downcast(&*item.read()))
+                mem::transmute::<&H, &H>(downcast(&*item.borrow()))
             }
         })
     }
 
     /// Get a mutable reference to the header field's value, if it exists.
     pub fn get_mut<H: Header + HeaderFormat>(&mut self) -> Option<&mut H> {
-        self.get_or_parse::<H>().map(|item| {
-            unsafe {
-                mem::transmute::<&mut H, &mut H>(downcast_mut(&mut *item.write()))
-            }
+        self.get_or_parse_mut::<H>().map(|item| {
+            unsafe { downcast_mut(item.borrow_mut()) }
         })
     }
 
-    fn get_or_parse<H: Header + HeaderFormat>(&self) -> Option<&RWLock<Item>> {
-        self.data.get(&CaseInsensitive(Borrowed(header_name::<H>()))).and_then(|item| get_or_parse::<H>(item))
+    fn get_or_parse<H: Header + HeaderFormat>(&self) -> Option<&MuCell<Item>> {
+        self.data.get(&CaseInsensitive(Borrowed(header_name::<H>()))).and_then(get_or_parse::<H>)
+    }
+
+    fn get_or_parse_mut<H: Header + HeaderFormat>(&mut self) -> Option<&mut MuCell<Item>> {
+        self.data.get_mut(&CaseInsensitive(Borrowed(header_name::<H>()))).and_then(get_or_parse_mut::<H>)
     }
 
     /// Returns a boolean of whether a certain header is in the map.
@@ -253,7 +259,7 @@ impl fmt::Show for Headers {
 
 /// An `Iterator` over the fields in a `Headers` map.
 pub struct HeadersItems<'a> {
-    inner: Entries<'a, CaseInsensitive, RWLock<Item>>
+    inner: Entries<'a, CaseInsensitive, MuCell<Item>>
 }
 
 impl<'a> Iterator<HeaderView<'a>> for HeadersItems<'a> {
@@ -266,7 +272,7 @@ impl<'a> Iterator<HeaderView<'a>> for HeadersItems<'a> {
 }
 
 /// Returned with the `HeadersItems` iterator.
-pub struct HeaderView<'a>(&'a CaseInsensitive, &'a RWLock<Item>);
+pub struct HeaderView<'a>(&'a CaseInsensitive, &'a MuCell<Item>);
 
 impl<'a> HeaderView<'a> {
     /// Check if a HeaderView is a certain Header.
@@ -286,21 +292,21 @@ impl<'a> HeaderView<'a> {
     pub fn value<H: Header + HeaderFormat>(&self) -> Option<&'a H> {
         get_or_parse::<H>(self.1).map(|item| {
             unsafe {
-                mem::transmute::<&H, &H>(downcast(&*item.read()))
+                mem::transmute::<&H, &H>(downcast(&*item.borrow()))
             }
         })
     }
 
     /// Get just the header value as a String.
     #[inline]
     pub fn value_string(&self) -> String {
-        (*self.1.read()).to_string()
+        (*self.1.borrow()).to_string()
     }
 }
 
 impl<'a> fmt::Show for HeaderView<'a> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}: {}", self.0, *self.1.read())
+        write!(f, "{}: {}", self.0, *self.1.borrow())
     }
 }
 
@@ -343,8 +349,8 @@ impl Item {
 
 }
 
-fn get_or_parse<H: Header + HeaderFormat>(item: &RWLock<Item>) -> Option<&RWLock<Item>> {
-    match item.read().typed {
+fn get_or_parse<H: Header + HeaderFormat>(item: &MuCell<Item>) -> Option<&MuCell<Item>> {
+    match item.borrow().typed {
         Some(ref typed) if typed.is::<H>() => return Some(item),
         Some(ref typed) => {
             warn!("attempted to access {} as wrong type", typed);
@@ -353,53 +359,55 @@ fn get_or_parse<H: Header + HeaderFormat>(item: &RWLock<Item>) -> Option<&RWLock
         _ => ()
     }
 
-    // Take out a write lock to do the parsing and mutation.
-    let mut write = item.write();
-
-    // Since this lock can queue, it's possible another thread just
-    // did the work for us.
-    match write.typed {
-        // Check they inserted the correct type and move on.
-        Some(ref typed) if typed.is::<H>() => return Some(item),
+    let worked = item.try_mutate(parse::<H>);
+    debug_assert!(worked, "item.try_mutate should return true");
+    if item.borrow().typed.is_some() {
+        Some(item)
+    } else {
+        None
+    }
+}
 
-        // Wrong type, another thread got here before us and parsed
-        // as a different representation.
+fn get_or_parse_mut<H: Header + HeaderFormat>(item: &mut MuCell<Item>) -> Option<&mut MuCell<Item>> {
+    let is_correct_type = match item.borrow().typed {
+        Some(ref typed) if typed.is::<H>() => Some(true),
         Some(ref typed) => {
-            debug!("other thread was here first?")
             warn!("attempted to access {} as wrong type", typed);
-            return None;
-        },
+            Some(false)
+        }
+        _ => None
+    };
 
-        // We are first in the queue or the only ones, so do the actual
-        // work of parsing and mutation.
-        _ => ()
+    match is_correct_type {
+        Some(true) => return Some(item),
+        Some(false) => return None,
+        None => ()
+    }
+
+    parse::<H>(item.borrow_mut());
+    if item.borrow().typed.is_some() {
+        Some(item)
+    } else {
+        None
     }
+}
 
-    let header = match write.raw {
+fn parse<H: Header + HeaderFormat>(item: &mut Item) {
+    item.typed = match item.raw {
         Some(ref raw) => match Header::parse_header(raw[]) {
-            Some::<H>(h) => h,
-            None => return None
+            Some::<H>(h) => Some(box h as Box<HeaderFormat + Send + Sync>),
+            None => None
         },
         None => unreachable!()
     };
-
-    // Mutate!
-    write.typed = Some(box header as Box<HeaderFormat + Send + Sync>);
-    Some(item)
 }
 
-fn downcast<H: Header + HeaderFormat>(read: &Item) -> &H {
-    match read.typed {
-        Some(ref val) => unsafe { val.downcast_ref_unchecked() },
-        _ => unreachable!()
-    }
+unsafe fn downcast<H: Header + HeaderFormat>(item: &Item) -> &H {
+    item.typed.as_ref().expect("item.typed must be set").downcast_ref_unchecked()
 }
 
-fn downcast_mut<H: Header + HeaderFormat>(write: &mut Item) -> &mut H {
-    match write.typed {
-        Some(ref mut val) => unsafe { val.downcast_mut_unchecked() },
-        _ => unreachable!()
-    }
+unsafe fn downcast_mut<H: Header + HeaderFormat>(item: &mut Item) -> &mut H {
+    item.typed.as_mut().expect("item.typed must be set").downcast_mut_unchecked()
 }
 
 impl fmt::Show for Item {
@@ -419,12 +427,6 @@ impl fmt::Show for Item {
     }
 }
 
-impl Clone for RWLock<Item> {
-    fn clone(&self) -> RWLock<Item> {
-        RWLock::new(self.read().clone())
-    }
-}
-
 impl fmt::Show for Box<HeaderFormat + Send + Sync> {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         (**self).fmt_header(fmt)
@@ -655,6 +657,32 @@ mod tests {
         }
     }
 
+    #[bench]
+    fn bench_header_get(b: &mut Bencher) {
+        let mut headers = Headers::new();
+        headers.set(ContentLength(11));
+        b.iter(|| assert_eq!(headers.get::<ContentLength>(), Some(&ContentLength(11))))
+    }
+
+    #[bench]
+    fn bench_header_get_miss(b: &mut Bencher) {
+        let headers = Headers::new();
+        b.iter(|| assert!(headers.get::<ContentLength>().is_none()))
+    }
+
+    #[bench]
+    fn bench_header_set(b: &mut Bencher) {
+        let mut headers = Headers::new();
+        b.iter(|| headers.set(ContentLength(12)))
+    }
+
+    #[bench]
+    fn bench_header_has(b: &mut Bencher) {
+        let mut headers = Headers::new();
+        headers.set(ContentLength(11));
+        b.iter(|| assert!(headers.has::<ContentLength>()))
+    }
+
     #[bench]
     fn bench_header_view_is(b: &mut Bencher) {
         let mut headers = Headers::new();
diff --git a/src/lib.rs b/src/lib.rs
@@ -134,6 +134,7 @@ extern crate openssl;
 #[cfg(test)] extern crate test;
 extern crate "unsafe-any" as uany;
 extern crate cookie;
+extern crate mucell;
 
 pub use std::io::net::ip::{SocketAddr, IpAddr, Ipv4Addr, Ipv6Addr, Port};
 pub use mimewrapper::mime;
