presentationAttributeCacheMaximumSize is set too low

https://bugs.webkit.org/show_bug.cgi?id=82816

Reviewed by Antti Koivisto.

Performance change, no side effects anticipated.

Prior to r106740, the presentation attribute style cache used to grow unbounded. As part of r110316,
a different version of the cache was introduced with a maximum size of 128. This size gave a 75% hit rate.

However this is bad as the previous cache had a 85% hit rate** and each miss propagate down to the matched
properties cache as we use pointer comparisons.

This change bumps the size to 4096. This size was chosen to bring us back to the previous hit rate while
being a power of 2 (it is the HashMap maximum size). The steep increase is due to the combinational compexity
of the new cache model: we need to match all our attributes' name, value and tag name to get a hit vs
one attribute name, value and a category in the previous cache. To avoid blowing up the memory, we introduced
a timer to clear the cache if the hit rate is too low.

The measured hit rate is actually better now - in the 90% range on most page cyclers. This ups the matched
properties hit rate by 1 percent point on presentation attributes.

** This is not a true apple-to-apple comparison as the cache model was changed.

* dom/StyledElement.cpp:
(PresentationAttributeCacheCleaner):
(WebCore::PresentationAttributeCacheCleaner::PresentationAttributeCacheCleaner):
(WebCore::PresentationAttributeCacheCleaner::didHitPresentationAttributeCache):
(WebCore::PresentationAttributeCacheCleaner::cleanCache):
(WebCore):
(WebCore::presentationAttributeCacheCleaner):
(WebCore::StyledElement::updateAttributeStyle):
Added the logic to clean the presentation attribute cache if we drop below 100 hits per minutes.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@113098 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 2c53906..05cbb8d 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2012-04-03  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        presentationAttributeCacheMaximumSize is set too low
+        https://bugs.webkit.org/show_bug.cgi?id=82816
+
+        Reviewed by Antti Koivisto.
+
+        Performance change, no side effects anticipated.
+
+        Prior to r106740, the presentation attribute style cache used to grow unbounded. As part of r110316,
+        a different version of the cache was introduced with a maximum size of 128. This size gave a 75% hit rate.
+
+        However this is bad as the previous cache had a 85% hit rate** and each miss propagate down to the matched
+        properties cache as we use pointer comparisons.
+
+        This change bumps the size to 4096. This size was chosen to bring us back to the previous hit rate while
+        being a power of 2 (it is the HashMap maximum size). The steep increase is due to the combinational compexity
+        of the new cache model: we need to match all our attributes' name, value and tag name to get a hit vs
+        one attribute name, value and a category in the previous cache. To avoid blowing up the memory, we introduced
+        a timer to clear the cache if the hit rate is too low.
+
+        The measured hit rate is actually better now - in the 90% range on most page cyclers. This ups the matched
+        properties hit rate by 1 percent point on presentation attributes.
+
+        ** This is not a true apple-to-apple comparison as the cache model was changed.
+
+        * dom/StyledElement.cpp:
+        (PresentationAttributeCacheCleaner):
+        (WebCore::PresentationAttributeCacheCleaner::PresentationAttributeCacheCleaner):
+        (WebCore::PresentationAttributeCacheCleaner::didHitPresentationAttributeCache):
+        (WebCore::PresentationAttributeCacheCleaner::cleanCache):
+        (WebCore):
+        (WebCore::presentationAttributeCacheCleaner):
+        (WebCore::StyledElement::updateAttributeStyle):
+        Added the logic to clean the presentation attribute cache if we drop below 100 hits per minutes.
+
 2012-04-03  Tony Chang  <tony@chromium.org>
 
         Implement new flex property and deprecate flex function

diff --git a/Source/WebCore/dom/StyledElement.cpp b/Source/WebCore/dom/StyledElement.cpp
index 6429204..1948872 100644 (file)
--- a/Source/WebCore/dom/StyledElement.cpp
+++ b/Source/WebCore/dom/StyledElement.cpp
@@ -74,6 +74,50 @@ static PresentationAttributeCache& presentationAttributeCache()
     return cache;
 }
 
+class PresentationAttributeCacheCleaner {
+    WTF_MAKE_NONCOPYABLE(PresentationAttributeCacheCleaner);
+public:
+    PresentationAttributeCacheCleaner()
+        : m_cleanTimer(this, &PresentationAttributeCacheCleaner::cleanCache)
+    {
+    }
+
+    void didHitPresentationAttributeCache()
+    {
+        if (presentationAttributeCache().size() < minimumPresentationAttributeCacheSizeForCleaning)
+            return;
+
+        m_hitCount++;
+
+        if (!m_cleanTimer.isActive())
+            m_cleanTimer.startOneShot(presentationAttributeCacheCleanTimeInSeconds);
+     }
+
+private:
+    static const unsigned presentationAttributeCacheCleanTimeInSeconds = 60;
+    static const int minimumPresentationAttributeCacheSizeForCleaning = 100;
+    static const unsigned minimumPresentationAttributeCacheHitCountPerMinute = (100 * presentationAttributeCacheCleanTimeInSeconds) / 60;
+
+    void cleanCache(Timer<PresentationAttributeCacheCleaner>* timer)
+    {
+        ASSERT_UNUSED(timer, timer == &m_cleanTimer);
+        unsigned hitCount = m_hitCount;
+        m_hitCount = 0;
+        if (hitCount > minimumPresentationAttributeCacheHitCountPerMinute)
+            return;
+        presentationAttributeCache().clear();
+    }
+
+    unsigned m_hitCount;
+    Timer<PresentationAttributeCacheCleaner> m_cleanTimer;
+};
+
+static PresentationAttributeCacheCleaner& presentationAttributeCacheCleaner()
+{
+    DEFINE_STATIC_LOCAL(PresentationAttributeCacheCleaner, cleaner, ());
+    return cleaner;
+}
+
 void StyledElement::updateStyleAttribute() const
 {
     ASSERT(!isStyleAttributeValid());
@@ -244,9 +288,10 @@ void StyledElement::updateAttributeStyle()
         cacheIterator = presentationAttributeCache().end();
 
     RefPtr<StylePropertySet> style;
-    if (cacheHash && cacheIterator->second) 
+    if (cacheHash && cacheIterator->second) {
         style = cacheIterator->second->value;
-    else {
+        presentationAttributeCacheCleaner().didHitPresentationAttributeCache();
+    } else {
         style = StylePropertySet::create(isSVGElement() ? SVGAttributeMode : CSSQuirksMode);
         unsigned size = attributeCount();
         for (unsigned i = 0; i < size; ++i) {
@@ -270,7 +315,7 @@ void StyledElement::updateAttributeStyle()
     newEntry->key = cacheKey;
     newEntry->value = style.release();
 
-    static const int presentationAttributeCacheMaximumSize = 128;
+    static const int presentationAttributeCacheMaximumSize = 4096;
     if (presentationAttributeCache().size() > presentationAttributeCacheMaximumSize) {
         // Start building from scratch if the cache ever gets big.
         presentationAttributeCache().clear();