Changeset 147889 in webkit

Timestamp:

Apr 7, 2013 6:01:22 PM (11 years ago)

Author:

benjamin@webkit.org

Message:

Do not allocate static AtomicStrings when searching for alternate font names
​https://bugs.webkit.org/show_bug.cgi?id=114110

Reviewed by Geoffrey Garen.

FontCache's alternateFamilyName does some font name substitutions
when a font is not found for a given name.

In the vast majority of cases, no substitutions are made and
alternateFamilyName returns the null atom.
Given the usage the function had a couple of problems:
-It created 6 static AtomicString.
-It did a case insensitive comparison with 6 differents strings

before failing.

This patch aims at solving both issues.

Instead of creating all AtomicString statically, the alternate name
are simply created on the fly as needed. Because the operation is so
unfrequent, and it uses the fast constructor, doing so should not slow
down the function.

To avoid comparing the input to every string, every time, the length of
the input is first evaluated, and we only do the necessary string comparisons.

On x86_64, this reduces the binary size by 2128 bytes.

• platform/graphics/FontCache.cpp:

(WebCore::familyNameEqualIgnoringCase):
When comparing strings, we know:
-The family name is non null.
-The length of both strings is equal.
-The length is not null.
Make the comparison accordingly.

(WebCore::alternateFamilyName):
(WebCore::FontCache::getCachedFontPlatformData):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/FontCache.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-04-07  Benjamin Poulain  <benjamin@webkit.org>
+
+        Do not allocate static AtomicStrings when searching for alternate font names
+        https://bugs.webkit.org/show_bug.cgi?id=114110
+
+        Reviewed by Geoffrey Garen.
+
+        FontCache's alternateFamilyName does some font name substitutions
+        when a font is not found for a given name.
+
+        In the vast majority of cases, no substitutions are made and
+        alternateFamilyName returns the null atom.
+        Given the usage the function had a couple of problems:
+        -It created 6 static AtomicString.
+        -It did a case insensitive comparison with 6 differents strings
+         before failing.
+
+        This patch aims at solving both issues.
+
+        Instead of creating all AtomicString statically, the alternate name
+        are simply created on the fly as needed. Because the operation is so
+        unfrequent, and it uses the fast constructor, doing so should not slow
+        down the function.
+
+        To avoid comparing the input to every string, every time, the length of
+        the input is first evaluated, and we only do the necessary string comparisons.
+
+        On x86_64, this reduces the binary size by 2128 bytes.
+
+        * platform/graphics/FontCache.cpp:
+        (WebCore::familyNameEqualIgnoringCase):
+        When comparing strings, we know:
+        -The family name is non null.
+        -The length of both strings is equal.
+        -The length is not null.
+        Make the comparison accordingly.
+
+        (WebCore::alternateFamilyName):
+        (WebCore::FontCache::getCachedFontPlatformData):
+
 2013-04-07  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebCore/platform/graphics/FontCache.cpp

@@ -133 +133 @@
 static FontPlatformDataCache* gFontPlatformDataCache = 0;
 
-static const AtomicString& alternateFamilyName(const AtomicString& familyName)
-{
-    // Alias Courier <-> Courier New
-    DEFINE_STATIC_LOCAL(AtomicString, courier, ("Courier", AtomicString::ConstructFromLiteral));
-    DEFINE_STATIC_LOCAL(AtomicString, courierNew, ("Courier New", AtomicString::ConstructFromLiteral));
-    if (equalIgnoringCase(familyName, courier))
-        return courierNew;
+static bool familyNameEqualIgnoringCase(const AtomicString& familyName, const char* reference, unsigned length)
+{
+    ASSERT(length > 0);
+    ASSERT(familyName.length() == length);
+    ASSERT(strlen(reference) == length);
+    const AtomicStringImpl* familyNameImpl = familyName.impl();
+    if (familyNameImpl->is8Bit())
+        return equalIgnoringCase(familyNameImpl->characters8(), reinterpret_cast<const LChar*>(reference), length);
+    return equalIgnoringCase(familyNameImpl->characters16(), reinterpret_cast<const LChar*>(reference), length);
+}
+
+template<size_t length>
+static inline bool familyNameEqualIgnoringCase(const AtomicString& familyName, const char (&reference)[length])
+{
+    return familyNameEqualIgnoringCase(familyName, reference, length - 1);
+}
+
+static const AtomicString alternateFamilyName(const AtomicString& familyName)
+{
+    // Alias Courier and Courier New.
+    // Alias Times and Times New Roman.
+    // Alias Arial and Helvetica.
+    switch (familyName.length()) {
+    case 5:
+        if (familyNameEqualIgnoringCase(familyName, "Arial"))
+            return AtomicString("Helvetica", AtomicString::ConstructFromLiteral);
+        if (familyNameEqualIgnoringCase(familyName, "Times"))
+            return AtomicString("Times New Roman", AtomicString::ConstructFromLiteral);
+        break;
+    case 7:
+        if (familyNameEqualIgnoringCase(familyName, "Courier"))
+            return AtomicString("Courier New", AtomicString::ConstructFromLiteral);
+        break;
+    case 9:
+        if (familyNameEqualIgnoringCase(familyName, "Helvetica"))
+            return AtomicString("Arial", AtomicString::ConstructFromLiteral);
+        break;
 #if !OS(WINDOWS)
     // On Windows, Courier New (truetype font) is always present and
     // Courier is a bitmap font. So, we don't want to map Courier New to
     // Courier.
-    if (equalIgnoringCase(familyName, courierNew))
-        return courier;
-#endif
-
-    // Alias Times and Times New Roman.
-    DEFINE_STATIC_LOCAL(AtomicString, times, ("Times", AtomicString::ConstructFromLiteral));
-    DEFINE_STATIC_LOCAL(AtomicString, timesNewRoman, ("Times New Roman", AtomicString::ConstructFromLiteral));
-    if (equalIgnoringCase(familyName, times))
-        return timesNewRoman;
-    if (equalIgnoringCase(familyName, timesNewRoman))
-        return times;
-    
-    // Alias Arial and Helvetica
-    DEFINE_STATIC_LOCAL(AtomicString, arial, ("Arial", AtomicString::ConstructFromLiteral));
-    DEFINE_STATIC_LOCAL(AtomicString, helvetica, ("Helvetica", AtomicString::ConstructFromLiteral));
-    if (equalIgnoringCase(familyName, arial))
-        return helvetica;
-    if (equalIgnoringCase(familyName, helvetica))
-        return arial;
-
+    case 11:
+        if (familyNameEqualIgnoringCase(familyName, "Courier New"))
+            return AtomicString("Courier", AtomicString::ConstructFromLiteral);
+        break;
+#endif // !OS(WINDOWS)
+    case 15:
+        if (familyNameEqualIgnoringCase(familyName, "Times New Roman"))
+            return AtomicString("Times", AtomicString::ConstructFromLiteral);
+        break;
 #if OS(WINDOWS)
     // On Windows, bitmap fonts are blocked altogether so that we have to 
     // alias MS Sans Serif (bitmap font) -> Microsoft Sans Serif (truetype font)
-    DEFINE_STATIC_LOCAL(AtomicString, msSans, ("MS Sans Serif", AtomicString::ConstructFromLiteral));
-    DEFINE_STATIC_LOCAL(AtomicString, microsoftSans, ("Microsoft Sans Serif", AtomicString::ConstructFromLiteral));
-    if (equalIgnoringCase(familyName, msSans))
-        return microsoftSans;
+    case 13:
+        if (familyNameEqualIgnoringCase(familyName, "MS Sans Serif"))
+            return AtomicString("Microsoft Sans Serif", AtomicString::ConstructFromLiteral);
+        break;
 
     // Alias MS Serif (bitmap) -> Times New Roman (truetype font). There's no 
     // 'Microsoft Sans Serif-equivalent' for Serif.
-    DEFINE_STATIC_LOCAL(AtomicString, msSerif, ("MS Serif", AtomicString::ConstructFromLiteral));
-    if (equalIgnoringCase(familyName, msSerif))
-        return timesNewRoman;
-#endif
+    case 8:
+        if (familyNameEqualIgnoringCase(familyName, "MS Serif"))
+            return AtomicString("Times New Roman", AtomicString::ConstructFromLiteral);
+        break;
+#endif // OS(WINDOWS)
+
+    }
 
     return nullAtom;
@@ -213 +235 @@
             // We were unable to find a font.  We have a small set of fonts that we alias to other names,
             // e.g., Arial/Helvetica, Courier/Courier New, etc.  Try looking up the font under the aliased name.
-            const AtomicString& alternateName = alternateFamilyName(familyName);
+            const AtomicString alternateName = alternateFamilyName(familyName);
             if (!alternateName.isNull()) {
                 FontPlatformData* fontPlatformDataForAlternateName = getCachedFontPlatformData(fontDescription, alternateName, true);