[Cocoa] FontPlatformData objects aren't cached at all when using font-family:system-ui
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Apr 2019 00:51:04 +0000 (00:51 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Apr 2019 00:51:04 +0000 (00:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196846
<rdar://problem/49499971>

Reviewed by Simon Fraser and Darin Adler.

PerformanceTests:

* Layout/system-ui-rebuild-emoji.html: Added.

Source/WebCore:

When adding the special codepath for system-ui to behave as an entire list of fonts rather than a single item,
I never added a cache for the FontPlatformData objects that codepath creates. The non-system-ui codepath already
has a cache in fontPlatformDataCache() in FontCache.cpp.

This patch causes a 16.8x performance improvement on the attached benchmark.

Test: PerformanceTests/Layout/system-ui-rebuild-emoji.html

* page/cocoa/MemoryReleaseCocoa.mm:
(WebCore::platformReleaseMemory):
* platform/graphics/cocoa/FontCacheCoreText.cpp:
(WebCore::invalidateFontCache):
* platform/graphics/cocoa/FontFamilySpecificationCoreText.cpp:
(WebCore::FontFamilySpecificationKey::FontFamilySpecificationKey):
(WebCore::FontFamilySpecificationKey::operator== const):
(WebCore::FontFamilySpecificationKey::operator!= const):
(WebCore::FontFamilySpecificationKey::isHashTableDeletedValue const):
(WebCore::FontFamilySpecificationKey::computeHash const):
(WebCore::FontFamilySpecificationKeyHash::hash):
(WebCore::FontFamilySpecificationKeyHash::equal):
(WebCore::fontMap):
(WebCore::clearFontFamilySpecificationCoreTextCache):
(WebCore::FontFamilySpecificationCoreText::fontRanges const):
* platform/graphics/cocoa/FontFamilySpecificationCoreText.h:
* platform/graphics/mac/ComplexTextControllerCoreText.mm:
(WebCore::ComplexTextController::collectComplexTextRunsForCharacters):
(WebCore::safeCFEqual): Deleted.

Source/WTF:

* wtf/RetainPtr.h:
(WTF::safeCFEqual):
(WTF::safeCFHash):

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

PerformanceTests/ChangeLog
PerformanceTests/Layout/system-ui-rebuild-emoji.html [new file with mode: 0644]
Source/WTF/ChangeLog
Source/WTF/wtf/RetainPtr.h
Source/WebCore/ChangeLog
Source/WebCore/page/cocoa/MemoryReleaseCocoa.mm
Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp
Source/WebCore/platform/graphics/cocoa/FontFamilySpecificationCoreText.cpp
Source/WebCore/platform/graphics/cocoa/FontFamilySpecificationCoreText.h
Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm

index 981537d..96ed6d3 100644 (file)
@@ -1,3 +1,13 @@
+2019-04-15  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        [Cocoa] FontPlatformData objects aren't cached at all when using font-family:system-ui
+        https://bugs.webkit.org/show_bug.cgi?id=196846
+        <rdar://problem/49499971>
+
+        Reviewed by Simon Fraser and Darin Adler.
+
+        * Layout/system-ui-rebuild-emoji.html: Added.
+
 2019-04-03  Myles C. Maxfield  <mmaxfield@apple.com>
 
         -apple-trailing-word is needed for browser detection
diff --git a/PerformanceTests/Layout/system-ui-rebuild-emoji.html b/PerformanceTests/Layout/system-ui-rebuild-emoji.html
new file mode 100644 (file)
index 0000000..39d3d1c
--- /dev/null
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+    <script src="../resources/runner.js"></script>
+<style>
+#target {
+    display: none;
+}
+</style>
+<style id="a" media="print">
+#target {
+    display: block;
+    width: 280px;
+}
+</style>
+<style id="b" media="print">
+#target {
+    display: block;
+    width: 300px;
+}
+</style>
+<style id="c" media="print">
+#target {
+    display: block;
+    width: 290px;
+}
+</style>
+</head>
+<body>
+    <pre id="log"></pre>
+    <div id="target" style="font-family: system-ui;">
+    <p>😎</p>
+    </div>
+    <script>
+        var target = document.getElementById("target");
+        var a = document.getElementById("a");
+        var b = document.getElementById("b");
+        var c = document.getElementById("c");
+
+        function test() {
+            a.media = "screen";
+            target.offsetLeft;
+            b.media = "screen";
+            target.offsetLeft;
+            c.media = "screen";
+            target.offsetLeft;
+            a.media = "print";
+            b.media = "print";
+            c.media = "print";
+        }
+
+        PerfTestRunner.measureRunsPerSecond({ run: test });
+    </script>
+</body>
+</html>
index 7a98b5a..95c8c5d 100644 (file)
@@ -1,3 +1,15 @@
+2019-04-15  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        [Cocoa] FontPlatformData objects aren't cached at all when using font-family:system-ui
+        https://bugs.webkit.org/show_bug.cgi?id=196846
+        <rdar://problem/49499971>
+
+        Reviewed by Simon Fraser and Darin Adler.
+
+        * wtf/RetainPtr.h:
+        (WTF::safeCFEqual):
+        (WTF::safeCFHash):
+
 2019-04-12  Ryosuke Niwa  <rniwa@webkit.org>
 
         HashTable::removeIf always shrinks the hash table by half even if there is nothing left
index e0eba2d..37ebbfe 100644 (file)
@@ -369,6 +369,16 @@ struct RetainPtrObjectHash {
     static const bool safeToCompareToEmptyOrDeleted = false;
 };
 
+inline bool safeCFEqual(CFTypeRef a, CFTypeRef b)
+{
+    return (!a && !b) || (a && b && CFEqual(a, b));
+}
+
+inline CFHashCode safeCFHash(CFTypeRef a)
+{
+    return a ? CFHash(a) : 0;
+}
+
 #ifdef __OBJC__
 template<typename T> T* dynamic_objc_cast(id object)
 {
index 77d250c..3f6c948 100644 (file)
@@ -1,3 +1,39 @@
+2019-04-15  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        [Cocoa] FontPlatformData objects aren't cached at all when using font-family:system-ui
+        https://bugs.webkit.org/show_bug.cgi?id=196846
+        <rdar://problem/49499971>
+
+        Reviewed by Simon Fraser and Darin Adler.
+
+        When adding the special codepath for system-ui to behave as an entire list of fonts rather than a single item,
+        I never added a cache for the FontPlatformData objects that codepath creates. The non-system-ui codepath already
+        has a cache in fontPlatformDataCache() in FontCache.cpp.
+
+        This patch causes a 16.8x performance improvement on the attached benchmark.
+
+        Test: PerformanceTests/Layout/system-ui-rebuild-emoji.html
+
+        * page/cocoa/MemoryReleaseCocoa.mm:
+        (WebCore::platformReleaseMemory):
+        * platform/graphics/cocoa/FontCacheCoreText.cpp:
+        (WebCore::invalidateFontCache):
+        * platform/graphics/cocoa/FontFamilySpecificationCoreText.cpp:
+        (WebCore::FontFamilySpecificationKey::FontFamilySpecificationKey):
+        (WebCore::FontFamilySpecificationKey::operator== const):
+        (WebCore::FontFamilySpecificationKey::operator!= const):
+        (WebCore::FontFamilySpecificationKey::isHashTableDeletedValue const):
+        (WebCore::FontFamilySpecificationKey::computeHash const):
+        (WebCore::FontFamilySpecificationKeyHash::hash):
+        (WebCore::FontFamilySpecificationKeyHash::equal):
+        (WebCore::fontMap):
+        (WebCore::clearFontFamilySpecificationCoreTextCache):
+        (WebCore::FontFamilySpecificationCoreText::fontRanges const):
+        * platform/graphics/cocoa/FontFamilySpecificationCoreText.h:
+        * platform/graphics/mac/ComplexTextControllerCoreText.mm:
+        (WebCore::ComplexTextController::collectComplexTextRunsForCharacters):
+        (WebCore::safeCFEqual): Deleted.
+
 2019-04-15  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: fake value descriptors for promises add a catch handler, preventing "rejectionhandled" events from being fired
index 0f4770b..5991ae4 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "MemoryRelease.h"
 
+#import "FontFamilySpecificationCoreText.h"
 #import "GCController.h"
 #import "IOSurfacePool.h"
 #import "LayerPool.h"
@@ -46,6 +47,7 @@ void platformReleaseMemory(Critical)
 #if USE_PLATFORM_SYSTEM_FALLBACK_LIST
     SystemFontDatabaseCoreText::singleton().clear();
 #endif
+    clearFontFamilySpecificationCoreTextCache();
 
 #if PLATFORM(IOS_FAMILY) && !PLATFORM(IOS_FAMILY_SIMULATOR) && !PLATFORM(IOSMAC)
     // FIXME: Remove this call to GSFontInitialize() once <rdar://problem/32886715> is fixed.
index 5778125..5f68b83 100644 (file)
@@ -27,6 +27,7 @@
 #include "FontCache.h"
 
 #include "Font.h"
+#include "FontFamilySpecificationCoreText.h"
 #include "SystemFontDatabaseCoreText.h"
 #include <pal/spi/cocoa/CoreTextSPI.h>
 
@@ -1204,6 +1205,7 @@ static void invalidateFontCache()
 #if USE_PLATFORM_SYSTEM_FALLBACK_LIST
     SystemFontDatabaseCoreText::singleton().clear();
 #endif
+    clearFontFamilySpecificationCoreTextCache();
 
     FontDatabase::singletonAllowingUserInstalledFonts().clear();
     FontDatabase::singletonDisallowingUserInstalledFonts().clear();
index 9e711e3..b749cda 100644 (file)
 #include "config.h"
 #include "FontFamilySpecificationCoreText.h"
 
-#include <pal/spi/cocoa/CoreTextSPI.h>
 #include "FontCache.h"
 #include "FontSelector.h"
+#include <pal/spi/cocoa/CoreTextSPI.h>
+#include <wtf/HashFunctions.h>
+#include <wtf/HashMap.h>
 
 #include <CoreText/CoreText.h>
 
 namespace WebCore {
 
+struct FontFamilySpecificationKey {
+    FontFamilySpecificationKey() = default;
+
+    FontFamilySpecificationKey(CTFontDescriptorRef fontDescriptor, const FontDescription& fontDescription)
+        : fontDescriptor(fontDescriptor)
+        , fontDescriptionKey(fontDescription)
+    { }
+
+    explicit FontFamilySpecificationKey(WTF::HashTableDeletedValueType deletedValue)
+        : fontDescriptionKey(deletedValue)
+    { }
+
+    bool operator==(const FontFamilySpecificationKey& other) const
+    {
+        return WTF::safeCFEqual(fontDescriptor.get(), other.fontDescriptor.get()) && fontDescriptionKey == other.fontDescriptionKey;
+    }
+
+    bool operator!=(const FontFamilySpecificationKey& other) const
+    {
+        return !(*this == other);
+    }
+
+    bool isHashTableDeletedValue() const { return fontDescriptionKey.isHashTableDeletedValue(); }
+
+    unsigned computeHash() const
+    {
+        return WTF::pairIntHash(WTF::safeCFHash(fontDescriptor.get()), fontDescriptionKey.computeHash());
+    }
+
+    RetainPtr<CTFontDescriptorRef> fontDescriptor;
+    FontDescriptionKey fontDescriptionKey;
+};
+
+struct FontFamilySpecificationKeyHash {
+    static unsigned hash(const FontFamilySpecificationKey& key) { return key.computeHash(); }
+    static bool equal(const FontFamilySpecificationKey& a, const FontFamilySpecificationKey& b) { return a == b; }
+    static const bool safeToCompareToEmptyOrDeleted = true;
+};
+
+using FontMap = HashMap<FontFamilySpecificationKey, std::unique_ptr<FontPlatformData>, FontFamilySpecificationKeyHash, WTF::SimpleClassHashTraits<FontFamilySpecificationKey>>;
+
+static FontMap& fontMap()
+{
+    static NeverDestroyed<FontMap> fontMap;
+    return fontMap;
+}
+
+void clearFontFamilySpecificationCoreTextCache()
+{
+    fontMap().clear();
+}
+
 FontFamilySpecificationCoreText::FontFamilySpecificationCoreText(CTFontDescriptorRef fontDescriptor)
     : m_fontDescriptor(fontDescriptor)
 {
@@ -43,24 +97,28 @@ FontFamilySpecificationCoreText::~FontFamilySpecificationCoreText() = default;
 
 FontRanges FontFamilySpecificationCoreText::fontRanges(const FontDescription& fontDescription) const
 {
-    auto size = fontDescription.computedSize();
+    auto& fontPlatformData = fontMap().ensure(FontFamilySpecificationKey(m_fontDescriptor.get(), fontDescription), [&] () {
+        auto size = fontDescription.computedSize();
 
-    auto font = adoptCF(CTFontCreateWithFontDescriptor(m_fontDescriptor.get(), size, nullptr));
+        auto font = adoptCF(CTFontCreateWithFontDescriptor(m_fontDescriptor.get(), size, nullptr));
 
-    auto fontForSynthesisComputation = font;
+        auto fontForSynthesisComputation = font;
 #if USE_PLATFORM_SYSTEM_FALLBACK_LIST
-    if (auto physicalFont = adoptCF(CTFontCopyPhysicalFont(font.get())))
-        fontForSynthesisComputation = physicalFont;
+        if (auto physicalFont = adoptCF(CTFontCopyPhysicalFont(font.get())))
+            fontForSynthesisComputation = physicalFont;
 #endif
 
-    font = preparePlatformFont(font.get(), fontDescription, nullptr, nullptr, { }, fontDescription.computedSize());
+        font = preparePlatformFont(font.get(), fontDescription, nullptr, nullptr, { }, fontDescription.computedSize());
+
+        bool syntheticBold, syntheticOblique;
+        std::tie(syntheticBold, syntheticOblique) = computeNecessarySynthesis(fontForSynthesisComputation.get(), fontDescription).boldObliquePair();
 
-    bool syntheticBold, syntheticOblique;
-    std::tie(syntheticBold, syntheticOblique) = computeNecessarySynthesis(fontForSynthesisComputation.get(), fontDescription).boldObliquePair();
+        return std::make_unique<FontPlatformData>(font.get(), size, false, syntheticOblique, fontDescription.orientation(), fontDescription.widthVariant(), fontDescription.textRenderingMode());
+    }).iterator->value;
 
-    FontPlatformData fontPlatformData(font.get(), size, false, syntheticOblique, fontDescription.orientation(), fontDescription.widthVariant(), fontDescription.textRenderingMode());
+    ASSERT(fontPlatformData);
 
-    return FontRanges(FontCache::singleton().fontForPlatformData(fontPlatformData));
+    return FontRanges(FontCache::singleton().fontForPlatformData(*fontPlatformData));
 }
 
 }
index 7474e62..900261a 100644 (file)
@@ -108,11 +108,6 @@ static const UniChar* provideStringAndAttributes(CFIndex stringIndex, CFIndex* c
     return info->cp + stringIndex;
 }
 
-static inline bool safeCFEqual(CFTypeRef a, CFTypeRef b)
-{
-    return (!a && !b) || (a && b && CFEqual(a, b));
-}
-
 void ComplexTextController::collectComplexTextRunsForCharacters(const UChar* cp, unsigned length, unsigned stringLocation, const Font* font)
 {
     if (!font) {
@@ -196,13 +191,13 @@ void ComplexTextController::collectComplexTextRunsForCharacters(const UChar* cp,
             CTFontRef runCTFont = static_cast<CTFontRef>(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
             ASSERT(runCTFont && CFGetTypeID(runCTFont) == CTFontGetTypeID());
             RetainPtr<CFTypeRef> runFontEqualityObject = FontPlatformData::objectForEqualityCheck(runCTFont);
-            if (!safeCFEqual(runFontEqualityObject.get(), font->platformData().objectForEqualityCheck().get())) {
+            if (!WTF::safeCFEqual(runFontEqualityObject.get(), font->platformData().objectForEqualityCheck().get())) {
                 // Begin trying to see if runFont matches any of the fonts in the fallback list.
                 for (unsigned i = 0; !m_font.fallbackRangesAt(i).isNull(); ++i) {
                     runFont = m_font.fallbackRangesAt(i).fontForCharacter(baseCharacter);
                     if (!runFont)
                         continue;
-                    if (safeCFEqual(runFont->platformData().objectForEqualityCheck().get(), runFontEqualityObject.get()))
+                    if (WTF::safeCFEqual(runFont->platformData().objectForEqualityCheck().get(), runFontEqualityObject.get()))
                         break;
                     runFont = nullptr;
                 }