RuleSet should use malloc rather than Vector

The lion's share of the memory used by the style resolver is in the RuleSet
objects. Prior to this CL, these objects were structured as a HashMap from
AtomicStrings to pointers to vectors of RuleData. This CL simplifies this
object graph by removing a layer of indirection. Now we just have a HashMap
from AtomicStrings to an array of RuleDatas.

Rather than use a length to terminate the iteration over the Vector, this CL
uses a bit in RuleData to mark the end of the array. Together with removing the
extra pointer, this CL saves 15 kB on Mobile Gmail.

R=eseidel,ojan

Review URL: https://chromiumcodereview.appspot.com/17616009

git-svn-id: svn://svn.chromium.org/blink/trunk@153042 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/css/ElementRuleCollector.cpp

@@ -206,57 +206,66 @@ inline bool ElementRuleCollector::ruleMatches(const RuleData& ruleData, const Co
     return true;
 }
 
+void ElementRuleCollector::collectRuleIfMatches(const RuleData& ruleData, const MatchRequest& matchRequest, StyleResolver::RuleRange& ruleRange)
+{
+    if (m_canUseFastReject && m_selectorFilter.fastRejectSelector<RuleData::maximumIdentifierCount>(ruleData.descendantSelectorIdentifierHashes()))
+        return;
+
+    StyleRule* rule = ruleData.rule();
+    InspectorInstrumentationCookie cookie = InspectorInstrumentation::willMatchRule(document(), rule, m_inspectorCSSOMWrappers, document()->styleSheetCollection());
+    PseudoId dynamicPseudo = NOPSEUDO;
+    if (ruleMatches(ruleData, matchRequest.scope, dynamicPseudo)) {
+        // If the rule has no properties to apply, then ignore it in the non-debug mode.
+        const StylePropertySet* properties = rule->properties();
+        if (!properties || (properties->isEmpty() && !matchRequest.includeEmptyRules)) {
+            InspectorInstrumentation::didMatchRule(cookie, false);
+            return;
+        }
+        // FIXME: Exposing the non-standard getMatchedCSSRules API to web is the only reason this is needed.
+        if (m_sameOriginOnly && !ruleData.hasDocumentSecurityOrigin()) {
+            InspectorInstrumentation::didMatchRule(cookie, false);
+            return;
+        }
+        // If we're matching normal rules, set a pseudo bit if
+        // we really just matched a pseudo-element.
+        if (dynamicPseudo != NOPSEUDO && m_pseudoStyleRequest.pseudoId == NOPSEUDO) {
+            if (m_mode == SelectorChecker::CollectingRules) {
+                InspectorInstrumentation::didMatchRule(cookie, false);
+                return;
+            }
+            if (dynamicPseudo < FIRST_INTERNAL_PSEUDOID)
+                m_state.style()->setHasPseudoStyle(dynamicPseudo);
+        } else {
+            // Update our first/last rule indices in the matched rules array.
+            ++ruleRange.lastRuleIndex;
+            if (ruleRange.firstRuleIndex == -1)
+                ruleRange.firstRuleIndex = ruleRange.lastRuleIndex;
+
+            // Add this rule to our list of matched rules.
+            addMatchedRule(&ruleData);
+            InspectorInstrumentation::didMatchRule(cookie, true);
+            return;
+        }
+    }
+    InspectorInstrumentation::didMatchRule(cookie, false);
+}
+
+void ElementRuleCollector::collectMatchingRulesForList(const RuleData* rules, const MatchRequest& matchRequest, StyleResolver::RuleRange& ruleRange)
+{
+    if (!rules)
+        return;
+    while (!rules->isLastInArray())
+        collectRuleIfMatches(*rules++, matchRequest, ruleRange);
+    collectRuleIfMatches(*rules, matchRequest, ruleRange);
+}
+
 void ElementRuleCollector::collectMatchingRulesForList(const Vector<RuleData>* rules, const MatchRequest& matchRequest, StyleResolver::RuleRange& ruleRange)
 {
     if (!rules)
         return;
-
-    const StyleResolverState& state = m_state;
-
     unsigned size = rules->size();
-    for (unsigned i = 0; i < size; ++i) {
-        const RuleData& ruleData = rules->at(i);
-        if (m_canUseFastReject && m_selectorFilter.fastRejectSelector<RuleData::maximumIdentifierCount>(ruleData.descendantSelectorIdentifierHashes()))
-            continue;
-
-        StyleRule* rule = ruleData.rule();
-        InspectorInstrumentationCookie cookie = InspectorInstrumentation::willMatchRule(document(), rule, m_inspectorCSSOMWrappers, document()->styleSheetCollection());
-        PseudoId dynamicPseudo = NOPSEUDO;
-        if (ruleMatches(ruleData, matchRequest.scope, dynamicPseudo)) {
-            // If the rule has no properties to apply, then ignore it in the non-debug mode.
-            const StylePropertySet* properties = rule->properties();
-            if (!properties || (properties->isEmpty() && !matchRequest.includeEmptyRules)) {
-                InspectorInstrumentation::didMatchRule(cookie, false);
-                continue;
-            }
-            // FIXME: Exposing the non-standard getMatchedCSSRules API to web is the only reason this is needed.
-            if (m_sameOriginOnly && !ruleData.hasDocumentSecurityOrigin()) {
-                InspectorInstrumentation::didMatchRule(cookie, false);
-                continue;
-            }
-            // If we're matching normal rules, set a pseudo bit if
-            // we really just matched a pseudo-element.
-            if (dynamicPseudo != NOPSEUDO && m_pseudoStyleRequest.pseudoId == NOPSEUDO) {
-                if (m_mode == SelectorChecker::CollectingRules) {
-                    InspectorInstrumentation::didMatchRule(cookie, false);
-                    continue;
-                }
-                if (dynamicPseudo < FIRST_INTERNAL_PSEUDOID)
-                    state.style()->setHasPseudoStyle(dynamicPseudo);
-            } else {
-                // Update our first/last rule indices in the matched rules array.
-                ++ruleRange.lastRuleIndex;
-                if (ruleRange.firstRuleIndex == -1)
-                    ruleRange.firstRuleIndex = ruleRange.lastRuleIndex;
-
-                // Add this rule to our list of matched rules.
-                addMatchedRule(&ruleData);
-                InspectorInstrumentation::didMatchRule(cookie, true);
-                continue;
-            }
-        }
-        InspectorInstrumentation::didMatchRule(cookie, false);
-    }
+    for (unsigned i = 0; i < size; ++i)
+        collectRuleIfMatches(rules->at(i), matchRequest, ruleRange);
 }
 
 static inline bool compareRules(const RuleData* r1, const RuleData* r2)

third_party/WebKit/Source/core/css/ElementRuleCollector.h

@@ -77,7 +77,9 @@ public:
 private:
     Document* document() { return m_state.document(); }
 
+    void collectRuleIfMatches(const RuleData&, const MatchRequest&, StyleResolver::RuleRange&);
     void collectMatchingRulesForList(const Vector<RuleData>*, const MatchRequest&, StyleResolver::RuleRange&);
+    void collectMatchingRulesForList(const RuleData*, const MatchRequest&, StyleResolver::RuleRange&);
     bool ruleMatches(const RuleData&, const ContainerNode* scope, PseudoId&);
 
     void sortMatchedRules();

third_party/WebKit/Source/core/css/RuleSet.cpp

@@ -130,9 +130,80 @@ static inline PropertyWhitelistType determinePropertyWhitelistType(const AddRule
     return PropertyWhitelistNone;
 }
 
+namespace {
+
+// FIXME: Should we move this class to WTF?
+template<typename T>
+class TerminatedArrayBuilder {
+public:
+    explicit TerminatedArrayBuilder(PassOwnPtr<T> array)
+        : m_array(array)
+        , m_count(0)
+        , m_capacity(0)
+    {
+        if (!m_array)
+            return;
+        for (T* item = m_array.get(); !item->isLastInArray(); ++item)
+            ++m_count;
+        ++m_count; // To count the last item itself.
+        m_capacity = m_count;
+    }
+
+    void grow(size_t count)
+    {
+        ASSERT(count);
+        if (!m_array) {
+            ASSERT(!m_count);
+            ASSERT(!m_capacity);
+            m_capacity = count;
+            m_array = adoptPtr(static_cast<T*>(fastMalloc(m_capacity * sizeof(T))));
+            return;
+        }
+        m_capacity += count;
+        m_array = adoptPtr(static_cast<T*>(fastRealloc(m_array.leakPtr(), m_capacity * sizeof(T))));
+        m_array.get()[m_count - 1].setLastInArray(false);
+    }
+
+    void append(const T& item)
+    {
+        RELEASE_ASSERT(m_count < m_capacity);
+        ASSERT(!item.isLastInArray());
+        m_array.get()[m_count++] = item;
+    }
+
+    PassOwnPtr<T> release()
+    {
+        RELEASE_ASSERT(m_count == m_capacity);
+        if (m_array)
+            m_array.get()[m_count - 1].setLastInArray(true);
+        assertValid();
+        return m_array.release();
+    }
+
+private:
+#ifndef NDEBUG
+    void assertValid()
+    {
+        for (size_t i = 0; i < m_count; ++i) {
+            bool isLastInArray = (i + 1 == m_count);
+            ASSERT(m_array.get()[i].isLastInArray() == isLastInArray);
+        }
+    }
+#else
+    void assertValid() { }
+#endif
+
+    OwnPtr<T> m_array;
+    size_t m_count;
+    size_t m_capacity;
+};
+
+}
+
 RuleData::RuleData(StyleRule* rule, unsigned selectorIndex, unsigned position, AddRuleFlags addRuleFlags)
     : m_rule(rule)
     , m_selectorIndex(selectorIndex)
+    , m_isLastInArray(false)
     , m_position(position)
     , m_hasFastCheckableSelector((addRuleFlags & RuleCanUseFastCheckSelector) && SelectorCheckerFastPath::canUse(selector()))
     , m_specificity(selector()->specificity())
@@ -381,21 +452,16 @@ void RuleSet::compactPendingRules(PendingRuleMap& pendingMap, CompactRuleMap& co
     PendingRuleMap::iterator end = pendingMap.end();
     for (PendingRuleMap::iterator it = pendingMap.begin(); it != end; ++it) {
         OwnPtr<LinkedStack<RuleData> > pendingRules = it->value.release();
-        size_t pendingSize = pendingRules->size();
-        ASSERT(pendingSize);
-
-        OwnPtr<Vector<RuleData> >& compactRules = compactMap.add(it->key, nullptr).iterator->value;
-        if (!compactRules) {
-            compactRules = adoptPtr(new Vector<RuleData>);
-            compactRules->reserveInitialCapacity(pendingSize);
-        } else {
-            compactRules->reserveCapacity(compactRules->size() + pendingSize);
-        }
+        CompactRuleMap::iterator compactRules = compactMap.add(it->key, nullptr).iterator;
 
+        TerminatedArrayBuilder<RuleData> builder(compactRules->value.release());
+        builder.grow(pendingRules->size());
         while (!pendingRules->isEmpty()) {
-            compactRules->append(pendingRules->peek());
+            builder.append(pendingRules->peek());
             pendingRules->pop();
         }
+
+        compactRules->value = builder.release();
     }
 }
 

third_party/WebKit/Source/core/css/RuleSet.h

@@ -51,6 +51,7 @@ class StyleRuleRegion;
 class StyleSheetContents;
 
 class RuleData {
+    NEW_DELETE_SAME_AS_MALLOC_FREE;
 public:
     RuleData(StyleRule*, unsigned selectorIndex, unsigned position, AddRuleFlags);
 
@@ -59,6 +60,9 @@ public:
     const CSSSelector* selector() const { return m_rule->selectorList().selectorAt(m_selectorIndex); }
     unsigned selectorIndex() const { return m_selectorIndex; }
 
+    bool isLastInArray() const { return m_isLastInArray; }
+    void setLastInArray(bool flag) { m_isLastInArray = flag; }
+
     bool hasFastCheckableSelector() const { return m_hasFastCheckableSelector; }
     bool hasMultipartSelector() const { return m_hasMultipartSelector; }
     bool hasRightmostSelectorMatchingHTMLBasedOnRuleHash() const { return m_hasRightmostSelectorMatchingHTMLBasedOnRuleHash; }
@@ -75,7 +79,8 @@ public:
 
 private:
     StyleRule* m_rule;
-    unsigned m_selectorIndex : 13;
+    unsigned m_selectorIndex : 12;
+    unsigned m_isLastInArray : 1; // We store an array of RuleData objects in a primitive array.
     // This number was picked fairly arbitrarily. We can probably lower it if we need to.
     // Some simple testing showed <100,000 RuleData's on large sites.
     unsigned m_position : 18;
@@ -111,10 +116,10 @@ public:
 
     const RuleFeatureSet& features() const { return m_features; }
 
-    const Vector<RuleData>* idRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_idRules.get(key); }
-    const Vector<RuleData>* classRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_classRules.get(key); }
-    const Vector<RuleData>* tagRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_tagRules.get(key); }
-    const Vector<RuleData>* shadowPseudoElementRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_shadowPseudoElementRules.get(key); }
+    const RuleData* idRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_idRules.get(key); }
+    const RuleData* classRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_classRules.get(key); }
+    const RuleData* tagRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_tagRules.get(key); }
+    const RuleData* shadowPseudoElementRules(AtomicStringImpl* key) const { ASSERT(!m_pendingRules); return m_shadowPseudoElementRules.get(key); }
     const Vector<RuleData>* linkPseudoClassRules() const { ASSERT(!m_pendingRules); return &m_linkPseudoClassRules; }
     const Vector<RuleData>* cuePseudoRules() const { ASSERT(!m_pendingRules); return &m_cuePseudoRules; }
     const Vector<RuleData>* focusPseudoClassRules() const { ASSERT(!m_pendingRules); return &m_focusPseudoClassRules; }
@@ -145,7 +150,7 @@ public:
 
 private:
     typedef HashMap<AtomicStringImpl*, OwnPtr<LinkedStack<RuleData> > > PendingRuleMap;
-    typedef HashMap<AtomicStringImpl*, OwnPtr<Vector<RuleData> > > CompactRuleMap;
+    typedef HashMap<AtomicStringImpl*, OwnPtr<RuleData> > CompactRuleMap;
 
     RuleSet()
         : m_ruleCount(0)