Changeset 107919 in webkit

Timestamp:

Feb 16, 2012 3:05:24 AM (12 years ago)

Author:

kling@webkit.org

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/CSSParser.cpp (modified) (5 diffs)
• css/CSSValuePool.cpp (modified) (1 diff)
• css/CSSValuePool.h (modified) (3 diffs)
• css/FontFamilyValue.cpp (modified) (1 diff)
• css/FontFamilyValue.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/css/CSSParser.cpp

@@ -4296 +4296 @@
 }
 
+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()
 {
@@ -4301 +4330 @@
     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)
@@ -4313 +4344 @@
 
         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 {
@@ -4345 +4377 @@
         if (nextValBreaksFont) {
             value = m_valueList->next();
-            currFamily = 0;
+            familyBuilder.commit();
+            inFamily = false;
         }
         else if (nextValIsFontName)
@@ -4352 +4385 @@
             break;
     }
+    familyBuilder.commit();
+
     if (!list->length())
         list = 0;

trunk/Source/WebCore/css/CSSValuePool.cpp

@@ -121 +121 @@
 }
 
+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;
 }
+
+}

trunk/Source/WebCore/css/CSSValuePool.h

@@ -30 +30 @@
 #include "CSSInitialValue.h"
 #include "CSSPrimitiveValue.h"
+#include "FontFamilyValue.h"
 #include <wtf/HashMap.h>
 #include <wtf/RefPtr.h>
@@ -40 +41 @@
     ~CSSValuePool();
 
+    PassRefPtr<FontFamilyValue> createFontFamilyValue(const String&);
     PassRefPtr<CSSInheritedValue> createInheritedValue() { return m_inheritedValue; }
     PassRefPtr<CSSInitialValue> createImplicitInitialValue() { return m_implicitInitialValue; }
@@ -72 +74 @@
     IntegerValueCache m_percentValueCache;
     IntegerValueCache m_numberValueCache;
+
+    typedef HashMap<String, RefPtr<FontFamilyValue> > FontFamilyValueCache;
+    FontFamilyValueCache m_fontFamilyValueCache;
 };
 

trunk/Source/WebCore/css/FontFamilyValue.cpp

@@ -59 +59 @@
 }
 
-void FontFamilyValue::appendSpaceSeparated(const UChar* characters, unsigned length)
-{
-    m_familyName.append(' ');
-    m_familyName.append(characters, length);
-}
-
 String FontFamilyValue::customCssText() const
 {

trunk/Source/WebCore/css/FontFamilyValue.h

@@ -34 +34 @@
     }
 
-    void appendSpaceSeparated(const UChar* characters, unsigned length);
-
     const String& familyName() const { return m_familyName; }