Only cache nth-indices when child count > 32.

When matching :nth-* selectors, we sparsely cache the child index count
into a hashmap for the parent element. Doing this regardlessly gave us
a performance penalty for small number of children as where noticed in
a performance degradation for [1].

The new approach is to not cache any indices until we match an :nth-*
selector for which we walk more than 32 siblings. The number 32 were
proposed in the bug report, and it turns out to be quite suitable given
the experiments which were done comparing the implementation not using
a cache at all, and the implementation where we cached regardlessly.

We trigger caching for nth-of-type indices based on the sibling count
as well, but not the sibling-of-type count as that would cause terrible
performance if the elements of the same type were sparse compared to
other siblings.

Gives a > 40% performance improvement for [1].

[1] blink_perf.css:PseudoClassSelectors.

BUG=483338
TEST=blink_perf.css:PseudoClassSelectors

Review URL: https://codereview.chromium.org/1655993005

Cr-Commit-Position: refs/heads/master@{#374356}

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

@@ -180,50 +180,6 @@ static bool isLastOfType(Element& element, const QualifiedName& type)
     return !ElementTraversal::nextSibling(element, HasTagName(type));
 }
 
-static int nthChildIndex(Element& element)
-{
-    if (NthIndexCache* nthIndexCache = element.document().nthIndexCache())
-        return nthIndexCache->nthChildIndex(element);
-
-    int index = 1;
-    for (const Element* sibling = ElementTraversal::previousSibling(element); sibling; sibling = ElementTraversal::previousSibling(*sibling))
-        index++;
-
-    return index;
-}
-
-static int nthOfTypeIndex(Element& element, const QualifiedName& type)
-{
-    if (NthIndexCache* nthIndexCache = element.document().nthIndexCache())
-        return nthIndexCache->nthChildIndexOfType(element, type);
-    int index = 1;
-    for (const Element* sibling = ElementTraversal::previousSibling(element, HasTagName(type)); sibling; sibling = ElementTraversal::previousSibling(*sibling, HasTagName(type)))
-        ++index;
-    return index;
-}
-
-static int nthLastChildIndex(Element& element)
-{
-    if (NthIndexCache* nthIndexCache = element.document().nthIndexCache())
-        return nthIndexCache->nthLastChildIndex(element);
-
-    int index = 1;
-    for (const Element* sibling = ElementTraversal::nextSibling(element); sibling; sibling = ElementTraversal::nextSibling(*sibling))
-        ++index;
-    return index;
-}
-
-static int nthLastOfTypeIndex(Element& element, const QualifiedName& type)
-{
-    if (NthIndexCache* nthIndexCache = element.document().nthIndexCache())
-        return nthIndexCache->nthLastChildIndexOfType(element, type);
-
-    int index = 1;
-    for (const Element* sibling = ElementTraversal::nextSibling(element, HasTagName(type)); sibling; sibling = ElementTraversal::nextSibling(*sibling, HasTagName(type)))
-        ++index;
-    return index;
-}
-
 // Recursive check of selectors and combinators
 // It can return 4 different values:
 // * SelectorMatches          - the selector matches the element e
@@ -770,14 +726,14 @@ bool SelectorChecker::checkPseudoClass(const SelectorCheckingContext& context, M
         if (ContainerNode* parent = element.parentElementOrDocumentFragment()) {
             if (m_mode == ResolvingStyle)
                 parent->setChildrenAffectedByForwardPositionalRules();
-            return selector.matchNth(nthChildIndex(element));
+            return selector.matchNth(NthIndexCache::nthChildIndex(element));
         }
         break;
     case CSSSelector::PseudoNthOfType:
         if (ContainerNode* parent = element.parentElementOrDocumentFragment()) {
             if (m_mode == ResolvingStyle)
                 parent->setChildrenAffectedByForwardPositionalRules();
-            return selector.matchNth(nthOfTypeIndex(element, element.tagQName()));
+            return selector.matchNth(NthIndexCache::nthOfTypeIndex(element));
         }
         break;
     case CSSSelector::PseudoNthLastChild:
@@ -786,7 +742,7 @@ bool SelectorChecker::checkPseudoClass(const SelectorCheckingContext& context, M
                 parent->setChildrenAffectedByBackwardPositionalRules();
             if (!parent->isFinishedParsingChildren())
                 return false;
-            return selector.matchNth(nthLastChildIndex(element));
+            return selector.matchNth(NthIndexCache::nthLastChildIndex(element));
         }
         break;
     case CSSSelector::PseudoNthLastOfType:
@@ -795,7 +751,7 @@ bool SelectorChecker::checkPseudoClass(const SelectorCheckingContext& context, M
                 parent->setChildrenAffectedByBackwardPositionalRules();
             if (!parent->isFinishedParsingChildren())
                 return false;
-            return selector.matchNth(nthLastOfTypeIndex(element, element.tagQName()));
+            return selector.matchNth(NthIndexCache::nthLastOfTypeIndex(element));
         }
         break;
     case CSSSelector::PseudoTarget:

third_party/WebKit/Source/core/dom/NthIndexCache.cpp

@@ -24,20 +24,146 @@ NthIndexCache::~NthIndexCache()
     m_document->setNthIndexCache(nullptr);
 }
 
-NthIndexData& NthIndexCache::ensureNthIndexDataFor(Node& parent)
+namespace {
+
+// Generating the cached nth-index counts when the number of children
+// exceeds this count. This number is picked based on testing
+// querySelectorAll for :nth-child(3n+2) and :nth-of-type(3n+2) on an
+// increasing number of children.
+
+const unsigned kCachedSiblingCountLimit = 32;
+
+unsigned uncachedNthChildIndex(Element& element)
+{
+    int index = 1;
+    for (const Element* sibling = ElementTraversal::previousSibling(element); sibling; sibling = ElementTraversal::previousSibling(*sibling))
+        index++;
+
+    return index;
+}
+
+unsigned uncachedNthLastChildIndex(Element& element)
+{
+    int index = 1;
+    for (const Element* sibling = ElementTraversal::nextSibling(element); sibling; sibling = ElementTraversal::nextSibling(*sibling))
+        ++index;
+    return index;
+}
+
+unsigned uncachedNthOfTypeIndex(Element& element, unsigned& siblingCount)
+{
+    int index = 1;
+    const QualifiedName& tag = element.tagQName();
+    for (const Element* sibling = ElementTraversal::previousSibling(element); sibling; sibling = ElementTraversal::previousSibling(*sibling)) {
+        if (sibling->tagQName() == tag)
+            ++index;
+        ++siblingCount;
+    }
+    return index;
+}
+
+unsigned uncachedNthLastOfTypeIndex(Element& element, unsigned& siblingCount)
+{
+    int index = 1;
+    const QualifiedName& tag = element.tagQName();
+    for (const Element* sibling = ElementTraversal::nextSibling(element); sibling; sibling = ElementTraversal::nextSibling(*sibling)) {
+        if (sibling->tagQName() == tag)
+            ++index;
+        ++siblingCount;
+    }
+    return index;
+}
+
+} // namespace
+
+unsigned NthIndexCache::nthChildIndex(Element& element)
+{
+    if (element.isPseudoElement())
+        return 1;
+    ASSERT(element.parentNode());
+    NthIndexCache* nthIndexCache = element.document().nthIndexCache();
+    NthIndexData* nthIndexData = nullptr;
+    if (nthIndexCache && nthIndexCache->m_parentMap)
+        nthIndexData = nthIndexCache->m_parentMap->get(element.parentNode());
+    if (nthIndexData)
+        return nthIndexData->nthIndex(element);
+    unsigned index = uncachedNthChildIndex(element);
+    if (nthIndexCache && index > kCachedSiblingCountLimit)
+        nthIndexCache->cacheNthIndexDataForParent(element);
+    return index;
+}
+
+unsigned NthIndexCache::nthLastChildIndex(Element& element)
+{
+    if (element.isPseudoElement())
+        return 1;
+    ASSERT(element.parentNode());
+    NthIndexCache* nthIndexCache = element.document().nthIndexCache();
+    NthIndexData* nthIndexData = nullptr;
+    if (nthIndexCache && nthIndexCache->m_parentMap)
+        nthIndexData = nthIndexCache->m_parentMap->get(element.parentNode());
+    if (nthIndexData)
+        return nthIndexData->nthLastIndex(element);
+    unsigned index = uncachedNthLastChildIndex(element);
+    if (nthIndexCache && index > kCachedSiblingCountLimit)
+        nthIndexCache->cacheNthIndexDataForParent(element);
+    return index;
+}
+
+NthIndexData* NthIndexCache::nthTypeIndexDataForParent(Element& element) const
+{
+    ASSERT(element.parentNode());
+    if (!m_parentMapForType)
+        return nullptr;
+    if (const IndexByType* map = m_parentMapForType->get(element.parentNode()))
+        return map->get(element.tagName());
+    return nullptr;
+}
+
+unsigned NthIndexCache::nthOfTypeIndex(Element& element)
+{
+    if (element.isPseudoElement())
+        return 1;
+    NthIndexCache* nthIndexCache = element.document().nthIndexCache();
+    if (nthIndexCache) {
+        if (NthIndexData* nthIndexData = nthIndexCache->nthTypeIndexDataForParent(element))
+            return nthIndexData->nthOfTypeIndex(element);
+    }
+    unsigned siblingCount = 0;
+    unsigned index = uncachedNthOfTypeIndex(element, siblingCount);
+    if (nthIndexCache && siblingCount > kCachedSiblingCountLimit)
+        nthIndexCache->cacheNthOfTypeIndexDataForParent(element);
+    return index;
+}
+
+unsigned NthIndexCache::nthLastOfTypeIndex(Element& element)
 {
+    if (element.isPseudoElement())
+        return 1;
+    NthIndexCache* nthIndexCache = element.document().nthIndexCache();
+    if (nthIndexCache) {
+        if (NthIndexData* nthIndexData = nthIndexCache->nthTypeIndexDataForParent(element))
+            return nthIndexData->nthLastOfTypeIndex(element);
+    }
+    unsigned siblingCount = 0;
+    unsigned index = uncachedNthLastOfTypeIndex(element, siblingCount);
+    if (nthIndexCache && siblingCount > kCachedSiblingCountLimit)
+        nthIndexCache->cacheNthOfTypeIndexDataForParent(element);
+    return index;
+}
+
+void NthIndexCache::cacheNthIndexDataForParent(Element& element)
+{
+    ASSERT(element.parentNode());
     if (!m_parentMap)
         m_parentMap = adoptPtrWillBeNoop(new ParentMap());
 
-    ParentMap::AddResult addResult = m_parentMap->add(&parent, nullptr);
-    if (addResult.isNewEntry)
-        addResult.storedValue->value = adoptPtrWillBeNoop(new NthIndexData());
-
-    ASSERT(addResult.storedValue->value);
-    return *addResult.storedValue->value;
+    ParentMap::AddResult addResult = m_parentMap->add(element.parentNode(), nullptr);
+    ASSERT(addResult.isNewEntry);
+    addResult.storedValue->value = adoptPtrWillBeNoop(new NthIndexData(*element.parentNode()));
 }
 
-NthIndexCache::IndexByType& NthIndexCache::ensureTypeIndexMap(Node& parent)
+NthIndexCache::IndexByType& NthIndexCache::ensureTypeIndexMap(ContainerNode& parent)
 {
     if (!m_parentMapForType)
         m_parentMapForType = adoptPtrWillBeNoop(new ParentMapForType());
@@ -50,20 +176,17 @@ NthIndexCache::IndexByType& NthIndexCache::ensureTypeIndexMap(Node& parent)
     return *addResult.storedValue->value;
 }
 
-NthIndexData& NthIndexCache::nthIndexDataWithTagName(Element& element)
+void NthIndexCache::cacheNthOfTypeIndexDataForParent(Element& element)
 {
+    ASSERT(element.parentNode());
     IndexByType::AddResult addResult = ensureTypeIndexMap(*element.parentNode()).add(element.tagName(), nullptr);
-    if (addResult.isNewEntry)
-        addResult.storedValue->value = adoptPtrWillBeNoop(new NthIndexData());
-    return *addResult.storedValue->value;
+    ASSERT(addResult.isNewEntry);
+    addResult.storedValue->value = adoptPtrWillBeNoop(new NthIndexData(*element.parentNode(), element.tagQName()));
 }
 
-unsigned NthIndexData::nthIndex(Element& element)
+unsigned NthIndexData::nthIndex(Element& element) const
 {
-    if (element.isPseudoElement())
-        return 1;
-    if (!m_count)
-        return cacheNthIndices(element);
+    ASSERT(!element.isPseudoElement());
 
     unsigned index = 0;
     for (Element* sibling = &element; sibling; sibling = ElementTraversal::previousSibling(*sibling), index++) {
@@ -74,81 +197,61 @@ unsigned NthIndexData::nthIndex(Element& element)
     return index;
 }
 
-unsigned NthIndexData::nthIndexOfType(Element& element, const QualifiedName& type)
+unsigned NthIndexData::nthOfTypeIndex(Element& element) const
 {
-    if (element.isPseudoElement())
-        return 1;
-    if (!m_count)
-        return cacheNthIndicesOfType(element, type);
+    ASSERT(!element.isPseudoElement());
+
     unsigned index = 0;
-    for (Element* sibling = &element; sibling; sibling = ElementTraversal::previousSibling(*sibling, HasTagName(type)), index++) {
+    for (Element* sibling = &element; sibling; sibling = ElementTraversal::previousSibling(*sibling, HasTagName(element.tagQName())), index++) {
         auto it = m_elementIndexMap.find(sibling);
         if (it != m_elementIndexMap.end())
             return it->value + index;
     }
     return index;
 }
 
-unsigned NthIndexData::nthLastIndex(Element& element)
+unsigned NthIndexData::nthLastIndex(Element& element) const
 {
-    if (element.isPseudoElement())
-        return 1;
-    unsigned index = nthIndex(element);
-    return m_count - index + 1;
+    return m_count - nthIndex(element) + 1;
 }
 
-unsigned NthIndexData::nthLastIndexOfType(Element& element, const QualifiedName& type)
+unsigned NthIndexData::nthLastOfTypeIndex(Element& element) const
 {
-    if (element.isPseudoElement())
-        return 1;
-    unsigned index = nthIndexOfType(element, type);
-    return m_count - index + 1;
+    return m_count - nthOfTypeIndex(element) + 1;
 }
 
-unsigned NthIndexData::cacheNthIndices(Element& element)
+NthIndexData::NthIndexData(ContainerNode& parent)
 {
-    ASSERT(!element.isPseudoElement());
-    ASSERT(m_elementIndexMap.isEmpty());
-    unsigned index = 0;
     // The frequency at which we cache the nth-index for a set of siblings.
     // A spread value of 3 means every third Element will have its nth-index cached.
     // Using a spread value > 1 is done to save memory. Looking up the nth-index will
     // still be done in constant time in terms of sibling count, at most 'spread'
     // elements will be traversed.
     const unsigned spread = 3;
     unsigned count = 0;
-    for (Element* sibling = ElementTraversal::firstChild(*element.parentNode()); sibling; sibling = ElementTraversal::nextSibling(*sibling)) {
+    for (Element* sibling = ElementTraversal::firstChild(parent); sibling; sibling = ElementTraversal::nextSibling(*sibling)) {
         if (!(++count % spread))
             m_elementIndexMap.add(sibling, count);
-        if (sibling == &element)
-            index = count;
     }
-    ASSERT(count && index);
+    ASSERT(count);
     m_count = count;
-    return index;
 }
 
-unsigned NthIndexData::cacheNthIndicesOfType(Element& element, const QualifiedName& type)
+NthIndexData::NthIndexData(ContainerNode& parent, const QualifiedName& type)
 {
-    ASSERT(!element.isPseudoElement());
-    ASSERT(m_elementIndexMap.isEmpty());
-    unsigned index = 0;
     // The frequency at which we cache the nth-index of type for a set of siblings.
     // A spread value of 3 means every third Element of its type will have its nth-index cached.
     // Using a spread value > 1 is done to save memory. Looking up the nth-index of its type will
     // still be done in less time, as most number of elements traversed
     // will be equal to find 'spread' elements in the sibling set.
     const unsigned spread = 3;
     unsigned count = 0;
-    for (Element* sibling = ElementTraversal::firstChild(*element.parentNode(), HasTagName(type)); sibling; sibling = ElementTraversal::nextSibling(*sibling, HasTagName(type))) {
+    for (Element* sibling = ElementTraversal::firstChild(parent, HasTagName(type)); sibling; sibling = ElementTraversal::nextSibling(*sibling, HasTagName(type))) {
         if (!(++count % spread))
             m_elementIndexMap.add(sibling, count);
-        if (sibling == &element)
-            index = count;
     }
-    ASSERT(count && index);
+    ASSERT(count);
     m_count = count;
-    return index;
 }
 
 DEFINE_TRACE(NthIndexData)