auto merge of #3830 : pcwalton/servo/persistent-list-floats, r=kmcallister

r? @kmcallister

components/layout/floats.rs

@@ -5,11 +5,11 @@
 use servo_util::geometry::Au;
 use servo_util::logical_geometry::WritingMode;
 use servo_util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize};
+use servo_util::persistent_list::PersistentList;
 use std::cmp::{max, min};
 use std::i32;
 use std::fmt;
 use style::computed_values::float;
-use sync::Arc;
 
 /// The kind of float: left or right.
 #[deriving(Clone, Encodable, Show)]
@@ -51,13 +51,10 @@ impl fmt::Show for Float {
 }
 
 /// Information about the floats next to a flow.
-///
-/// FIXME(pcwalton): When we have fast `MutexArc`s, try removing `#[deriving(Clone)]` and wrap in a
-/// mutex.
 #[deriving(Clone)]
 struct FloatList {
     /// Information about each of the floats here.
-    floats: Vec<Float>,
+    floats: PersistentList<Float>,
     /// Cached copy of the maximum block-start offset of the float.
     max_block_start: Au,
 }
@@ -65,54 +62,21 @@ struct FloatList {
 impl FloatList {
     fn new() -> FloatList {
         FloatList {
-            floats: vec!(),
+            floats: PersistentList::new(),
             max_block_start: Au(0),
         }
     }
-}
-
-impl fmt::Show for FloatList {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "max_block_start={} floats={:?}", self.max_block_start, self.floats)
-    }
-}
-
-/// Wraps a `FloatList` to avoid allocation in the common case of no floats.
-///
-/// FIXME(pcwalton): When we have fast `MutexArc`s, try removing `CowArc` and use a mutex instead.
-#[deriving(Clone)]
-struct FloatListRef {
-    list: Option<Arc<FloatList>>,
-}
-
-impl FloatListRef {
-    fn new() -> FloatListRef {
-        FloatListRef {
-            list: None,
-        }
-    }
 
     /// Returns true if the list is allocated and false otherwise. If false, there are guaranteed
     /// not to be any floats.
     fn is_present(&self) -> bool {
-        self.list.is_some()
+        self.floats.len() > 0
     }
+}
 
-    #[inline]
+impl fmt::Show for FloatList {
-    fn get<'a>(&'a self) -> Option<&'a FloatList> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self.list {
+        write!(f, "max_block_start={} floats={}", self.max_block_start, self.floats.len())
-            None => None,
-            Some(ref list) => Some(&**list),
-        }
-    }
-
-    #[allow(experimental)]
-    #[inline]
-    fn get_mut<'a>(&'a mut self) -> &'a mut FloatList {
-        if self.list.is_none() {
-            self.list = Some(Arc::new(FloatList::new()))
-        }
-        self.list.as_mut().unwrap().make_unique()
     }
 }
 
@@ -130,7 +94,12 @@ pub struct PlacementInfo {
 
 impl fmt::Show for PlacementInfo {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "size={} ceiling={} max_inline_size={} kind={:?}", self.size, self.ceiling, self.max_inline_size, self.kind)
+        write!(f,
+               "size={} ceiling={} max_inline_size={} kind={:?}",
+               self.size,
+               self.ceiling,
+               self.max_inline_size,
+               self.kind)
     }
 }
 
@@ -143,21 +112,19 @@ fn range_intersect(block_start_1: Au, block_end_1: Au, block_start_2: Au, block_
 #[deriving(Clone)]
 pub struct Floats {
     /// The list of floats.
-    list: FloatListRef,
+    list: FloatList,
     /// The offset of the flow relative to the first float.
     offset: LogicalSize<Au>,
+    /// The writing mode of these floats.
     pub writing_mode: WritingMode,
 }
 
 impl fmt::Show for Floats {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self.list.get() {
+        if !self.list.is_present() {
-            None => {
             write!(f, "[empty]")
-            }
+        } else {
-            Some(list) => {
+            write!(f, "offset={} floats={}", self.offset, self.list)
-                write!(f, "offset={} floats={}", self.offset, list)
-            }
         }
     }
 }
@@ -166,7 +133,7 @@ impl Floats {
     /// Creates a new `Floats` object.
     pub fn new(writing_mode: WritingMode) -> Floats {
         Floats {
-            list: FloatListRef::new(),
+            list: FloatList::new(),
             offset: LogicalSize::zero(writing_mode),
             writing_mode: writing_mode,
         }
@@ -179,30 +146,23 @@ impl Floats {
 
     /// Returns the position of the last float in flow coordinates.
     pub fn last_float_pos(&self) -> Option<LogicalPoint<Au>> {
-        match self.list.get() {
+        match self.list.floats.front() {
-            None => None,
-            Some(list) => {
-                match list.floats.last() {
             None => None,
             Some(float) => Some(float.bounds.start + self.offset),
         }
     }
-        }
-    }
 
     pub fn len(&self) -> uint {
-        self.list.list.as_ref().map(|list| list.floats.len()).unwrap_or(0)
+        self.list.floats.len()
     }
 
-    /// Returns a rectangle that encloses the region from block-start to block-start + block-size, with inline-size small
+    /// Returns a rectangle that encloses the region from block-start to block-start + block-size,
-    /// enough that it doesn't collide with any floats. max_x is the x-coordinate beyond which
+    /// with inline-size small enough that it doesn't collide with any floats. max_x is the
-    /// floats have no effect. (Generally this is the containing block inline-size.)
+    /// inline-size beyond which floats have no effect. (Generally this is the containing block
-    pub fn available_rect(&self, block_start: Au, block_size: Au, max_x: Au) -> Option<LogicalRect<Au>> {
+    /// inline-size.)
-        let list = match self.list.get() {
+    pub fn available_rect(&self, block_start: Au, block_size: Au, max_x: Au)
-            None => return None,
+                          -> Option<LogicalRect<Au>> {
-            Some(list) => list,
+        let list = &self.list;
-        };
-
         let block_start = block_start - self.offset.block;
 
         debug!("available_rect: trying to find space at {}", block_start);
@@ -225,22 +185,26 @@ impl Floats {
             debug!("float_pos: {}, float_size: {}", float_pos, float_size);
             match float.kind {
                 FloatLeft if float_pos.i + float_size.inline > max_inline_start &&
-                        float_pos.b + float_size.block > block_start && float_pos.b < block_start + block_size => {
+                        float_pos.b + float_size.block > block_start &&
+                        float_pos.b < block_start + block_size => {
                     max_inline_start = float_pos.i + float_size.inline;
 
                     l_block_start = Some(float_pos.b);
                     l_block_end = Some(float_pos.b + float_size.block);
 
-                    debug!("available_rect: collision with inline_start float: new max_inline_start is {}",
+                    debug!("available_rect: collision with inline_start float: new \
+                            max_inline_start is {}",
                            max_inline_start);
                 }
                 FloatRight if float_pos.i < min_inline_end &&
-                       float_pos.b + float_size.block > block_start && float_pos.b < block_start + block_size => {
+                       float_pos.b + float_size.block > block_start &&
+                       float_pos.b < block_start + block_size => {
                     min_inline_end = float_pos.i;
 
                     r_block_start = Some(float_pos.b);
                     r_block_end = Some(float_pos.b + float_size.block);
-                    debug!("available_rect: collision with inline_end float: new min_inline_end is {}",
+                    debug!("available_rect: collision with inline_end float: new min_inline_end \
+                            is {}",
                             min_inline_end);
                 }
                 FloatLeft | FloatRight => {}
@@ -251,37 +215,47 @@ impl Floats {
         // If there are floats on both sides, take the intersection of the
         // two areas. Also make sure we never return a block-start smaller than the
         // given upper bound.
-        let (block_start, block_end) = match (r_block_start, r_block_end, l_block_start, l_block_end) {
+        let (block_start, block_end) = match (r_block_start,
-            (Some(r_block_start), Some(r_block_end), Some(l_block_start), Some(l_block_end)) =>
+                                              r_block_end,
-                range_intersect(max(block_start, r_block_start), r_block_end, max(block_start, l_block_start), l_block_end),
+                                              l_block_start,
-
+                                              l_block_end) {
-            (None, None, Some(l_block_start), Some(l_block_end)) => (max(block_start, l_block_start), l_block_end),
+            (Some(r_block_start), Some(r_block_end), Some(l_block_start), Some(l_block_end)) => {
-            (Some(r_block_start), Some(r_block_end), None, None) => (max(block_start, r_block_start), r_block_end),
+                range_intersect(max(block_start, r_block_start),
+                                r_block_end,
+                                max(block_start, l_block_start),
+                                l_block_end)
+            }
+            (None, None, Some(l_block_start), Some(l_block_end)) => {
+                (max(block_start, l_block_start), l_block_end)
+            }
+            (Some(r_block_start), Some(r_block_end), None, None) => {
+                (max(block_start, r_block_start), r_block_end)
+            }
             (None, None, None, None) => return None,
             _ => fail!("Reached unreachable state when computing float area")
         };
 
         // FIXME(eatkinson): This assertion is too strong and fails in some cases. It is OK to
-        // return negative inline-sizes since we check against that inline-end away, but we should still
+        // return negative inline-sizes since we check against that inline-end away, but we should
-        // undersrtand why they occur and add a stronger assertion here.
+        // still undersrtand why they occur and add a stronger assertion here.
         // assert!(max_inline-start < min_inline-end);
 
         assert!(block_start <= block_end, "Float position error");
 
-        Some(LogicalRect::new(
+        Some(LogicalRect::new(self.writing_mode,
-            self.writing_mode, max_inline_start + self.offset.inline, block_start + self.offset.block,
+                              max_inline_start + self.offset.inline,
-            min_inline_end - max_inline_start, block_end - block_start
+                              block_start + self.offset.block,
-        ))
+                              min_inline_end - max_inline_start,
+                              block_end - block_start))
     }
 
     /// Adds a new float to the list.
     pub fn add_float(&mut self, info: &PlacementInfo) {
         let new_info;
         {
-            let list = self.list.get_mut();
             new_info = PlacementInfo {
                 size: info.size,
-                ceiling: max(info.ceiling, list.max_block_start + self.offset.block),
+                ceiling: max(info.ceiling, self.list.max_block_start + self.offset.block),
                 max_inline_size: info.max_inline_size,
                 kind: info.kind
             }
@@ -298,18 +272,16 @@ impl Floats {
             kind: info.kind
         };
 
-        let list = self.list.get_mut();
+        self.list.floats = self.list.floats.prepend_elem(new_float);
-        list.floats.push(new_float);
+        self.list.max_block_start = max(self.list.max_block_start, new_float.bounds.start.b);
-        list.max_block_start = max(list.max_block_start, new_float.bounds.start.b);
     }
 
-    /// Given the block-start 3 sides of the rectangle, finds the largest block-size that will result in the
+    /// Given the three sides of the bounding rectangle in the block-start direction, finds the
-    /// rectangle not colliding with any floats. Returns None if that block-size is infinite.
+    /// largest block-size that will result in the rectangle not colliding with any floats. Returns
-    fn max_block_size_for_bounds(&self, inline_start: Au, block_start: Au, inline_size: Au) -> Option<Au> {
+    /// `None` if that block-size is infinite.
-        let list = match self.list.get() {
+    fn max_block_size_for_bounds(&self, inline_start: Au, block_start: Au, inline_size: Au)
-            None => return None,
+                                 -> Option<Au> {
-            Some(list) => list,
+        let list = &self.list;
-        };
 
         let block_start = block_start - self.offset.block;
         let inline_start = inline_start - self.offset.inline;
@@ -357,7 +329,9 @@ impl Floats {
         // Can't go any higher than previous floats or previous elements in the document.
         let mut float_b = info.ceiling;
         loop {
-            let maybe_location = self.available_rect(float_b, info.size.block, info.max_inline_size);
+            let maybe_location = self.available_rect(float_b,
+                                                     info.size.block,
+                                                     info.max_inline_size);
             debug!("place_float: Got available rect: {:?} for y-pos: {}", maybe_location, float_b);
             match maybe_location {
                 // If there are no floats blocking us, return the current location
@@ -421,11 +395,7 @@ impl Floats {
     }
 
     pub fn clearance(&self, clear: ClearType) -> Au {
-        let list = match self.list.get() {
+        let list = &self.list;
-            None => return Au(0),
-            Some(list) => list,
-        };
-
         let mut clearance = Au(0);
         for float in list.floats.iter() {
             match (clear, float.kind) {

components/util/lib.rs

@@ -45,6 +45,7 @@ pub mod logical_geometry;
 pub mod memory;
 pub mod namespace;
 pub mod opts;
+pub mod persistent_list;
 pub mod range;
 pub mod resource_files;
 pub mod rtinstrument;

components/util/persistent_list.rs

@@ -0,0 +1,100 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+//! A persistent, thread-safe singly-linked list.
+
+use std::mem;
+use sync::Arc;
+
+pub struct PersistentList<T> {
+    head: PersistentListLink<T>,
+    length: uint,
+}
+
+struct PersistentListEntry<T> {
+    value: T,
+    next: PersistentListLink<T>,
+}
+
+type PersistentListLink<T> = Option<Arc<PersistentListEntry<T>>>;
+
+impl<T> PersistentList<T> where T: Send + Sync {
+    #[inline]
+    pub fn new() -> PersistentList<T> {
+        PersistentList {
+            head: None,
+            length: 0,
+        }
+    }
+
+    #[inline]
+    pub fn len(&self) -> uint {
+        self.length
+    }
+
+    #[inline]
+    pub fn front(&self) -> Option<&T> {
+        self.head.as_ref().map(|head| &head.value)
+    }
+
+    #[inline]
+    pub fn prepend_elem(&self, value: T) -> PersistentList<T> {
+        PersistentList {
+            head: Some(Arc::new(PersistentListEntry {
+                value: value,
+                next: self.head.clone(),
+            })),
+            length: self.length + 1,
+        }
+    }
+
+    #[inline]
+    pub fn iter<'a>(&'a self) -> PersistentListIterator<'a,T> {
+        // This could clone (and would not need the lifetime if it did), but then it would incur
+        // atomic operations on every call to `.next()`. Bad.
+        PersistentListIterator {
+            entry: self.head.as_ref().map(|head| &**head),
+        }
+    }
+}
+
+impl<T> Clone for PersistentList<T> where T: Send + Sync {
+    fn clone(&self) -> PersistentList<T> {
+        // This establishes the persistent nature of this list: we can clone a list by just cloning
+        // its head.
+        PersistentList {
+            head: self.head.clone(),
+            length: self.length,
+        }
+    }
+}
+
+pub struct PersistentListIterator<'a,T> where T: 'a + Send + Sync {
+    entry: Option<&'a PersistentListEntry<T>>,
+}
+
+impl<'a,T> Iterator<&'a T> for PersistentListIterator<'a,T> where T: Send + Sync {
+    #[inline]
+    fn next(&mut self) -> Option<&'a T> {
+        let entry = match self.entry {
+            None => return None,
+            Some(entry) => {
+                // This `transmute` is necessary to ensure that the lifetimes of the next entry and
+                // this entry match up; the compiler doesn't know this, but we do because of the
+                // reference counting behavior of `Arc`.
+                unsafe {
+                    mem::transmute::<&'a PersistentListEntry<T>,
+                                     &'static PersistentListEntry<T>>(entry)
+                }
+            }
+        };
+        let value = &entry.value;
+        self.entry = match entry.next {
+            None => None,
+            Some(ref entry) => Some(&**entry),
+        };
+        Some(value)
+    }
+}
+