Title: [104550] trunk/Source/WebCore
Revision
104550
Author
[email protected]
Date
2012-01-10 03:35:32 -0800 (Tue, 10 Jan 2012)

Log Message

Matched declaration cache should support mapped attribute declarations.
<http://webkit.org/b/75948>

Reviewed by Antti Koivisto.

Support caching of matches that include mapped attribute declarations.
To make this possible, let the cache hold references to the matched style declarations.
Otherwise, declarations are assumed to survive until ~CSSStyleSelector, which may not
be the case for CSSMappedAttributeDeclaration.

To avoid hoarding of stale CSSMappedAttributeDeclarations in the cache, do a sweep for
every 100 additions and garbage collect any entries containing a declaration that is
only referenced by the cache.

This increases cache hit rate by up to ~10% on the Alexa top sites.

* css/CSSStyleSelector.cpp:
(WebCore::CSSStyleSelector::CSSStyleSelector):
(WebCore::CSSStyleSelector::sweepMatchedDeclarationCache):
(WebCore::CSSStyleSelector::matchAllRules):
(WebCore::CSSStyleSelector::applyDeclarations):
(WebCore::CSSStyleSelector::addToMatchedDeclarationCache):
* css/CSSStyleSelector.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (104549 => 104550)


--- trunk/Source/WebCore/ChangeLog	2012-01-10 11:33:37 UTC (rev 104549)
+++ trunk/Source/WebCore/ChangeLog	2012-01-10 11:35:32 UTC (rev 104550)
@@ -1,3 +1,29 @@
+2012-01-10  Andreas Kling  <[email protected]>
+
+        Matched declaration cache should support mapped attribute declarations.
+        <http://webkit.org/b/75948>
+
+        Reviewed by Antti Koivisto.
+
+        Support caching of matches that include mapped attribute declarations.
+        To make this possible, let the cache hold references to the matched style declarations.
+        Otherwise, declarations are assumed to survive until ~CSSStyleSelector, which may not
+        be the case for CSSMappedAttributeDeclaration.
+
+        To avoid hoarding of stale CSSMappedAttributeDeclarations in the cache, do a sweep for
+        every 100 additions and garbage collect any entries containing a declaration that is
+        only referenced by the cache.
+
+        This increases cache hit rate by up to ~10% on the Alexa top sites.
+
+        * css/CSSStyleSelector.cpp:
+        (WebCore::CSSStyleSelector::CSSStyleSelector):
+        (WebCore::CSSStyleSelector::sweepMatchedDeclarationCache):
+        (WebCore::CSSStyleSelector::matchAllRules):
+        (WebCore::CSSStyleSelector::applyDeclarations):
+        (WebCore::CSSStyleSelector::addToMatchedDeclarationCache):
+        * css/CSSStyleSelector.h:
+
 2012-01-10  Kenichi Ishibashi  <[email protected]>
 
         Complex path should be used when UVS exists in text run

Modified: trunk/Source/WebCore/css/CSSStyleSelector.cpp (104549 => 104550)


--- trunk/Source/WebCore/css/CSSStyleSelector.cpp	2012-01-10 11:33:37 UTC (rev 104549)
+++ trunk/Source/WebCore/css/CSSStyleSelector.cpp	2012-01-10 11:35:32 UTC (rev 104550)
@@ -330,6 +330,7 @@
                                    bool strictParsing, bool matchAuthorAndUserStyles)
     : m_hasUAAppearance(false)
     , m_backgroundData(BackgroundFillLayer)
+    , m_matchedDeclarationCacheAdditionsSinceLastSweep(0)
     , m_checker(document, strictParsing)
     , m_parentStyle(0)
     , m_rootElementStyle(0)
@@ -481,6 +482,27 @@
     m_fontSelector->clearDocument();
 }
 
+void CSSStyleSelector::sweepMatchedDeclarationCache()
+{
+    // Look for cache entries containing a style declaration with a single ref and remove them.
+    // This may happen when an element attribute mutation causes it to swap out its Attribute::decl()
+    // for another CSSMappedAttributeDeclaration, potentially leaving this cache with the last ref.
+    Vector<unsigned, 16> toRemove;
+    MatchedStyleDeclarationCache::iterator it = m_matchedStyleDeclarationCache.begin();
+    MatchedStyleDeclarationCache::iterator end = m_matchedStyleDeclarationCache.end();
+    for (; it != end; ++it) {
+        Vector<MatchedStyleDeclaration>& matchedStyleDeclarations = it->second.matchedStyleDeclarations;
+        for (size_t i = 0; i < matchedStyleDeclarations.size(); ++i) {
+            if (matchedStyleDeclarations[i].styleDeclaration->hasOneRef()) {
+                toRemove.append(it->first);
+                break;
+            }
+        }
+    }
+    for (size_t i = 0; i < toRemove.size(); ++i)
+        m_matchedStyleDeclarationCache.remove(toRemove[i]);
+}
+
 CSSStyleSelector::Features::Features()
     : usesFirstLineRules(false)
     , usesBeforeAfterRules(false)
@@ -626,12 +648,6 @@
 #endif
 }
 
-CSSStyleSelector::MatchedStyleDeclaration::MatchedStyleDeclaration() 
-{  
-    // Make sure all memory is zero initializes as we calculate hash over the bytes of this object.
-    memset(this, 0, sizeof(*this));
-}
-
 void CSSStyleSelector::addMatchedDeclaration(CSSMutableStyleDeclaration* styleDeclaration, unsigned linkMatchType)
 {
     m_matchedDecls.grow(m_matchedDecls.size() + 1);
@@ -820,7 +836,6 @@
                     if (result.firstAuthorRule == -1)
                         result.firstAuthorRule = result.lastAuthorRule;
                     addMatchedDeclaration(attr->decl());
-                    result.isCacheable = false;
                 }
             }
         }
@@ -2239,7 +2254,7 @@
 
     if (m_style->insideLink() != NotInsideLink) {
         for (int i = startIndex; i <= endIndex; ++i) {
-            CSSMutableStyleDeclaration* styleDeclaration = m_matchedDecls[i].styleDeclaration;
+            CSSMutableStyleDeclaration* styleDeclaration = m_matchedDecls[i].styleDeclaration.get();
             unsigned linkMatchType = m_matchedDecls[i].linkMatchType;
             // FIXME: It would be nicer to pass these as arguments but that requires changes in many places.
             m_applyPropertyToRegularStyle = linkMatchType & SelectorChecker::MatchLink;
@@ -2252,7 +2267,7 @@
         return;
     }
     for (int i = startIndex; i <= endIndex; ++i)
-        applyDeclaration<applyFirst>(m_matchedDecls[i].styleDeclaration, isImportant, inheritedOnly);
+        applyDeclaration<applyFirst>(m_matchedDecls[i].styleDeclaration.get(), isImportant, inheritedOnly);
 }
 
 unsigned CSSStyleSelector::computeDeclarationHash(MatchedStyleDeclaration* declarations, unsigned size)
@@ -2310,6 +2325,12 @@
 
 void CSSStyleSelector::addToMatchedDeclarationCache(const RenderStyle* style, const RenderStyle* parentStyle, unsigned hash, const MatchResult& matchResult)
 {
+    static unsigned matchedDeclarationCacheAdditionsBetweenSweeps = 100;
+    if (++m_matchedDeclarationCacheAdditionsSinceLastSweep >= matchedDeclarationCacheAdditionsBetweenSweeps) {
+        sweepMatchedDeclarationCache();
+        m_matchedDeclarationCacheAdditionsSinceLastSweep = 0;
+    }
+
     ASSERT(hash);
     MatchedStyleDeclarationCacheItem cacheItem;
     cacheItem.matchedStyleDeclarations.append(m_matchedDecls);

Modified: trunk/Source/WebCore/css/CSSStyleSelector.h (104549 => 104550)


--- trunk/Source/WebCore/css/CSSStyleSelector.h	2012-01-10 11:33:37 UTC (rev 104549)
+++ trunk/Source/WebCore/css/CSSStyleSelector.h	2012-01-10 11:35:32 UTC (rev 104550)
@@ -346,9 +346,14 @@
     void loadPendingImages();
 
     struct MatchedStyleDeclaration {
-        MatchedStyleDeclaration();
-        CSSMutableStyleDeclaration* styleDeclaration;
-        unsigned linkMatchType;
+        MatchedStyleDeclaration() : possiblyPaddedMember(0) { }
+
+        RefPtr<CSSMutableStyleDeclaration> styleDeclaration;
+        union {
+            unsigned linkMatchType;
+            // Used to make sure all memory is zero-initialized since we compute the hash over the bytes of this object.
+            void* possiblyPaddedMember;
+        };
     };
     static unsigned computeDeclarationHash(MatchedStyleDeclaration*, unsigned size);
     struct MatchedStyleDeclarationCacheItem {
@@ -360,11 +365,16 @@
     const MatchedStyleDeclarationCacheItem* findFromMatchedDeclarationCache(unsigned hash, const MatchResult&);
     void addToMatchedDeclarationCache(const RenderStyle*, const RenderStyle* parentStyle, unsigned hash, const MatchResult&);
 
+    // Every N additions to the matched declaration cache trigger a sweep where entries holding
+    // the last reference to a style declaration are garbage collected.
+    void sweepMatchedDeclarationCache();
+
     // We collect the set of decls that match in |m_matchedDecls|. We then walk the
     // set of matched decls four times, once for those properties that others depend on (like font-size),
     // and then a second time for all the remaining properties. We then do the same two passes
     // for any !important rules.
     Vector<MatchedStyleDeclaration, 64> m_matchedDecls;
+    unsigned m_matchedDeclarationCacheAdditionsSinceLastSweep;
 
     typedef HashMap<unsigned, MatchedStyleDeclarationCacheItem> MatchedStyleDeclarationCache;
     MatchedStyleDeclarationCache m_matchedStyleDeclarationCache;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to