Share font-family CSS values through CSSValuePool.
authorkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2012 11:05:24 +0000 (11:05 +0000)
committerkling@webkit.org <kling@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2012 11:05:24 +0000 (11:05 +0000)
<http://webkit.org/b/78604>

Reviewed by Darin Adler.

Cache and share FontFamilyValue instances in the per-document CSSValuePool.
This reduces memory consumption by 248 kB on the Moz page cycler (64-bit)
and avoids a bunch of extra work.

This is a regression from the recent attribute style refactoring; previously
the mapped attribute declaration table would ensure that multiple 'font'
elements with the same 'face' value would share the same FontFamilyValue.

We're not yet sharing the entire CSSValueList returned by parseFontFamily()
but this is a step on the way there.

* css/FontFamilyValue.cpp:
* css/FontFamilyValue.h:

    Removed appendSpaceSeparated(), making FontFamilyValue immutable.

* css/CSSParser.cpp:
(FontFamilyValueBuilder):
(WebCore::FontFamilyValueBuilder::FontFamilyValueBuilder):
(WebCore::FontFamilyValueBuilder::add):
(WebCore::FontFamilyValueBuilder::commit):
(WebCore::CSSParser::parseFontFamily):

    Refactor parseFontFamily() to defer creation of FontFamilyValue until
    the whole family name is known. Added a little helper class to avoid
    code duplication.

* css/CSSValuePool.h:
* css/CSSValuePool.cpp:
(WebCore::CSSValuePool::createFontFamilyValue):

    Added a FontFamilyValue cache to CSSValuePool. All values are tied to
    the lifetime of the pool.

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

Source/WebCore/ChangeLog
Source/WebCore/css/CSSParser.cpp
Source/WebCore/css/CSSValuePool.cpp
Source/WebCore/css/CSSValuePool.h
Source/WebCore/css/FontFamilyValue.cpp
Source/WebCore/css/FontFamilyValue.h

index d936879..b7db7ab 100644 (file)
@@ -1,3 +1,44 @@
+2012-02-15  Andreas Kling  <awesomekling@apple.com>
+
+        Share font-family CSS values through CSSValuePool.
+        <http://webkit.org/b/78604>
+
+        Reviewed by Darin Adler.
+
+        Cache and share FontFamilyValue instances in the per-document CSSValuePool.
+        This reduces memory consumption by 248 kB on the Moz page cycler (64-bit)
+        and avoids a bunch of extra work.
+
+        This is a regression from the recent attribute style refactoring; previously
+        the mapped attribute declaration table would ensure that multiple 'font'
+        elements with the same 'face' value would share the same FontFamilyValue.
+
+        We're not yet sharing the entire CSSValueList returned by parseFontFamily()
+        but this is a step on the way there.
+
+        * css/FontFamilyValue.cpp:
+        * css/FontFamilyValue.h:
+
+            Removed appendSpaceSeparated(), making FontFamilyValue immutable.
+
+        * css/CSSParser.cpp:
+        (FontFamilyValueBuilder):
+        (WebCore::FontFamilyValueBuilder::FontFamilyValueBuilder):
+        (WebCore::FontFamilyValueBuilder::add):
+        (WebCore::FontFamilyValueBuilder::commit):
+        (WebCore::CSSParser::parseFontFamily):
+
+            Refactor parseFontFamily() to defer creation of FontFamilyValue until
+            the whole family name is known. Added a little helper class to avoid
+            code duplication.
+
+        * css/CSSValuePool.h:
+        * css/CSSValuePool.cpp:
+        (WebCore::CSSValuePool::createFontFamilyValue):
+
+            Added a FontFamilyValue cache to CSSValuePool. All values are tied to
+            the lifetime of the pool.
+
 2012-02-16  Simon Hausmann  <simon.hausmann@nokia.com>
 
         [Qt] Move event conversion functions from WebCore to WebKit
index 7acfbb0..1b472a2 100644 (file)
@@ -4295,12 +4295,43 @@ bool CSSParser::parseFont(bool important)
     return true;
 }
 
+class FontFamilyValueBuilder {
+public:
+    FontFamilyValueBuilder(CSSValueList* list, CSSValuePool* pool)
+        : m_list(list)
+        , m_cssValuePool(pool)
+    {
+    }
+
+    void add(const CSSParserString& string)
+    {
+        if (!m_builder.isEmpty())
+            m_builder.append(' ');
+        m_builder.append(string.characters, string.length);
+    }
+
+    void commit()
+    {
+        if (m_builder.isEmpty())
+            return;
+        m_list->append(m_cssValuePool->createFontFamilyValue(m_builder.toString()));
+        m_builder.clear();
+    }
+
+private:
+    StringBuilder m_builder;
+    CSSValueList* m_list;
+    CSSValuePool* m_cssValuePool;
+};
+
 PassRefPtr<CSSValueList> CSSParser::parseFontFamily()
 {
     RefPtr<CSSValueList> list = CSSValueList::createCommaSeparated();
     CSSParserValue* value = m_valueList->current();
 
-    FontFamilyValue* currFamily = 0;
+    FontFamilyValueBuilder familyBuilder(list.get(), cssValuePool());
+    bool inFamily = false;
+
     while (value) {
         if (value->id == CSSValueInitial || value->id == CSSValueInherit)
             return 0;
@@ -4312,28 +4343,29 @@ PassRefPtr<CSSValueList> CSSParser::parseFontFamily()
             (nextValue->unit == CSSPrimitiveValue::CSS_STRING || nextValue->unit == CSSPrimitiveValue::CSS_IDENT));
 
         if (value->id >= CSSValueSerif && value->id <= CSSValueWebkitBody) {
-            if (currFamily)
-                currFamily->appendSpaceSeparated(value->string.characters, value->string.length);
+            if (inFamily)
+                familyBuilder.add(value->string);
             else if (nextValBreaksFont || !nextValIsFontName)
                 list->append(cssValuePool()->createIdentifierValue(value->id));
             else {
-                RefPtr<FontFamilyValue> newFamily = FontFamilyValue::create(value->string);
-                currFamily = newFamily.get();
-                list->append(newFamily.release());
+                familyBuilder.commit();
+                familyBuilder.add(value->string);
+                inFamily = true;
             }
         } else if (value->unit == CSSPrimitiveValue::CSS_STRING) {
             // Strings never share in a family name.
-            currFamily = 0;
-            list->append(FontFamilyValue::create(value->string));
+            inFamily = false;
+            familyBuilder.commit();
+            list->append(cssValuePool()->createFontFamilyValue(value->string));
         } else if (value->unit == CSSPrimitiveValue::CSS_IDENT) {
-            if (currFamily)
-                currFamily->appendSpaceSeparated(value->string.characters, value->string.length);
+            if (inFamily)
+                familyBuilder.add(value->string);
             else if (nextValBreaksFont || !nextValIsFontName)
-                list->append(FontFamilyValue::create(value->string));
+                list->append(cssValuePool()->createFontFamilyValue(value->string));
             else {
-                RefPtr<FontFamilyValue> newFamily = FontFamilyValue::create(value->string);
-                currFamily = newFamily.get();
-                list->append(newFamily.release());
+                familyBuilder.commit();
+                familyBuilder.add(value->string);
+                inFamily = true;
             }
         } else {
             break;
@@ -4344,13 +4376,16 @@ PassRefPtr<CSSValueList> CSSParser::parseFontFamily()
 
         if (nextValBreaksFont) {
             value = m_valueList->next();
-            currFamily = 0;
+            familyBuilder.commit();
+            inFamily = false;
         }
         else if (nextValIsFontName)
             value = nextValue;
         else
             break;
     }
+    familyBuilder.commit();
+
     if (!list->length())
         list = 0;
     return list.release();
index 3d8893e..acd65a9 100644 (file)
@@ -120,4 +120,12 @@ PassRefPtr<CSSPrimitiveValue> CSSValuePool::createValue(double value, CSSPrimiti
     return entry.first->second;
 }
 
+PassRefPtr<FontFamilyValue> CSSValuePool::createFontFamilyValue(const String& familyName)
+{
+    RefPtr<FontFamilyValue>& value = m_fontFamilyValueCache.add(familyName, 0).first->second;
+    if (!value)
+        value = FontFamilyValue::create(familyName);
+    return value;
+}
+
 }
index bc751eb..1140fee 100644 (file)
@@ -29,6 +29,7 @@
 #include "CSSInheritedValue.h"
 #include "CSSInitialValue.h"
 #include "CSSPrimitiveValue.h"
+#include "FontFamilyValue.h"
 #include <wtf/HashMap.h>
 #include <wtf/RefPtr.h>
 
@@ -39,6 +40,7 @@ public:
     static PassRefPtr<CSSValuePool> create() { return adoptRef(new CSSValuePool); }
     ~CSSValuePool();
 
+    PassRefPtr<FontFamilyValue> createFontFamilyValue(const String&);
     PassRefPtr<CSSInheritedValue> createInheritedValue() { return m_inheritedValue; }
     PassRefPtr<CSSInitialValue> createImplicitInitialValue() { return m_implicitInitialValue; }
     PassRefPtr<CSSInitialValue> createExplicitInitialValue() { return m_explicitInitialValue; }
@@ -71,6 +73,9 @@ private:
     IntegerValueCache m_pixelValueCache;
     IntegerValueCache m_percentValueCache;
     IntegerValueCache m_numberValueCache;
+
+    typedef HashMap<String, RefPtr<FontFamilyValue> > FontFamilyValueCache;
+    FontFamilyValueCache m_fontFamilyValueCache;
 };
 
 }
index 665895d..ffd71a7 100644 (file)
@@ -58,12 +58,6 @@ FontFamilyValue::FontFamilyValue(const String& familyName)
     m_familyName.truncate(length);
 }
 
-void FontFamilyValue::appendSpaceSeparated(const UChar* characters, unsigned length)
-{
-    m_familyName.append(' ');
-    m_familyName.append(characters, length);
-}
-
 String FontFamilyValue::customCssText() const
 {
     return quoteCSSStringIfNeeded(m_familyName);
index b2fbaa2..2f91b6e 100644 (file)
@@ -33,8 +33,6 @@ public:
         return adoptRef(new FontFamilyValue(familyName));
     }
 
-    void appendSpaceSeparated(const UChar* characters, unsigned length);
-
     const String& familyName() const { return m_familyName; }
 
     String customCssText() const;