Bug 1355343: Cache snapshot lookups. r?bholley draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 07 May 2017 02:36:19 +0200
changeset 574009 56b72bcfa20c1c7d2222891317ecb4eed34bfb98
parent 574008 49bb7a266caba0b18999ae9518d5e1be153d379b
child 574010 d7fc28f1403a8556efebce7f7f3bdc257890daba
child 574101 c388ce6be8b1948d9d853edd9c1db90263a46d64
child 574184 4d0ac2b96771e6d375807157e1c8156986f1d407
push id57555
push userbmo:emilio+bugs@crisal.io
push dateMon, 08 May 2017 08:06:12 +0000
reviewersbholley
bugs1355343
milestone55.0a1
Bug 1355343: Cache snapshot lookups. r?bholley This is likely to be important at least for the initial element. For the rest of them this is likely useless because we create them in a throwaway fashion. MozReview-Commit-ID: EFz9WUdB8S0 Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
servo/components/style/restyle_hints.rs
--- a/servo/components/style/restyle_hints.rs
+++ b/servo/components/style/restyle_hints.rs
@@ -17,16 +17,17 @@ use selector_parser::{AttrValue, NonTSPs
 use selectors::{Element, MatchAttr};
 use selectors::matching::{ElementSelectorFlags, StyleRelations};
 use selectors::matching::matches_selector;
 use selectors::parser::{AttrSelector, Combinator, Component, Selector};
 use selectors::parser::{SelectorInner, SelectorMethods};
 use selectors::visitor::SelectorVisitor;
 use smallvec::SmallVec;
 use std::borrow::Borrow;
+use std::cell::Cell;
 use std::clone::Clone;
 use stylist::SelectorMap;
 
 bitflags! {
     /// When the ElementState of an element (like IN_HOVER_STATE) changes,
     /// certain pseudo-classes (like :hover) may require us to restyle that
     /// element, its siblings, and/or its descendants. Similarly, when various
     /// attributes of an element change, we may also need to restyle things with
@@ -189,38 +190,47 @@ pub trait ElementSnapshot : Sized + Matc
     fn each_class<F>(&self, F)
         where F: FnMut(&Atom);
 }
 
 struct ElementWrapper<'a, E>
     where E: TElement,
 {
     element: E,
+    cached_snapshot: Cell<Option<&'a Snapshot>>,
     snapshot_map: &'a SnapshotMap,
 }
 
 impl<'a, E> ElementWrapper<'a, E>
     where E: TElement,
 {
     /// Trivially constructs an `ElementWrapper`.
     fn new(el: E, snapshot_map: &'a SnapshotMap) -> Self {
-        ElementWrapper { element: el, snapshot_map: snapshot_map }
+        ElementWrapper {
+            element: el,
+            cached_snapshot: Cell::new(None),
+            snapshot_map: snapshot_map,
+        }
     }
 
     /// Gets the snapshot associated with this element, if any.
-    ///
-    /// TODO(emilio): If the hash table lookup happens to appear in profiles, we
-    /// can cache the snapshot in a RefCell<&'a Snapshot>.
     fn snapshot(&self) -> Option<&'a Snapshot> {
         if !self.element.has_snapshot() {
-            return None
+            return None;
+        }
+
+        if let Some(s) = self.cached_snapshot.get() {
+            return Some(s);
         }
 
         let snapshot = self.snapshot_map.get(&self.element);
         debug_assert!(snapshot.is_some(), "has_snapshot lied!");
+
+        self.cached_snapshot.set(snapshot);
+
         snapshot
     }
 }
 
 impl<'a, E> MatchAttr for ElementWrapper<'a, E>
     where E: TElement,
 {
     type Impl = SelectorImpl;