REGRESSION(r254534): 1-3% regression on PLT
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Feb 2020 23:31:17 +0000 (23:31 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Feb 2020 23:31:17 +0000 (23:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207244
<rdar://problem/58821709>

Reviewed by Simon Fraser.

Source/WebCore:

Turns out CTFontTransformGlyphsWithLanguage() is 33.7% slower than CTFontTransformGlyphs() on some OSes.
This patch changes the preprocessor guards to not use the function on those OSes.
Also, the contract of the function on the more performant OSes requires that the locale name be canonicalized,
so this patch implements a canonical locale cache inside LocaleMac.mm. It gets cleared when the low memory
warning is fired.

Marked existing tests as failing.

* page/cocoa/MemoryReleaseCocoa.mm:
(WebCore::platformReleaseMemory):
* platform/graphics/cocoa/FontCocoa.mm:
(WebCore::Font::applyTransforms const):
* platform/graphics/mac/SimpleFontDataCoreText.cpp:
(WebCore::Font::getCFStringAttributes const):
* platform/text/mac/LocaleMac.h:
* platform/text/mac/LocaleMac.mm:
(WebCore::determineLocale):
(WebCore::canonicalLocaleMap):
(WebCore::LocaleMac::canonicalLanguageIdentifierFromString):
(WebCore::LocaleMac::releaseMemory):

Source/WTF:

CTFontTransformGlyphsWithLanguage() is only acceptable on certain OSes.

* wtf/PlatformHave.h:

LayoutTests:

Mark the tests as failing on certain OSes.

* platform/ios/TestExpectations:
* platform/mac/TestExpectations:

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/ios/TestExpectations
LayoutTests/platform/mac/TestExpectations
Source/WTF/ChangeLog
Source/WTF/wtf/PlatformHave.h
Source/WTF/wtf/PlatformUse.h
Source/WebCore/ChangeLog
Source/WebCore/page/cocoa/MemoryReleaseCocoa.mm
Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp
Source/WebCore/platform/text/mac/LocaleMac.h
Source/WebCore/platform/text/mac/LocaleMac.mm

index 063f7e5..105b4b1 100644 (file)
@@ -1,3 +1,16 @@
+2020-02-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION(r254534): 1-3% regression on PLT
+        https://bugs.webkit.org/show_bug.cgi?id=207244
+        <rdar://problem/58821709>
+
+        Reviewed by Simon Fraser.
+
+        Mark the tests as failing on certain OSes.
+
+        * platform/ios/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2020-02-06  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: show JavaScript Worker terminated state as an internal property
index 847f16a..d2bac8d 100644 (file)
@@ -3480,4 +3480,8 @@ webkit.org/b/207225 imported/w3c/web-platform-tests/service-workers/service-work
 
 webkit.org/b/207230 imported/w3c/web-platform-tests/fetch/stale-while-revalidate/fetch.html [ Pass Failure ]
 
-webkit.org/b/207278 imported/w3c/web-platform-tests/web-animations/timing-model/animations/finishing-an-animation.html [ Pass Failure ]
\ No newline at end of file
+webkit.org/b/207278 imported/w3c/web-platform-tests/web-animations/timing-model/animations/finishing-an-animation.html [ Pass Failure ]
+
+# Locale-specific shaping is only enabled on certain OSes.
+webkit.org/b/77568 fast/text/locale-shaping.html [ ImageOnlyFailure ]
+webkit.org/b/77568 fast/text/locale-shaping-complex.html [ ImageOnlyFailure ]
index b7938e6..07ce551 100644 (file)
@@ -1931,8 +1931,8 @@ webkit.org/b/200043 [ Sierra HighSierra Mojave Catalina ] fast/text/internationa
 webkit.org/b/204312 imported/w3c/web-platform-tests/svg/import/struct-dom-06-b-manual.svg [ Failure Pass ]
 
 # Locale-specific shaping is only enabled on certain OSes.
-webkit.org/b/77568 [ Sierra HighSierra Mojave ] fast/text/locale-shaping.html [ ImageOnlyFailure ]
-webkit.org/b/77568 [ Sierra HighSierra Mojave ] fast/text/locale-shaping-complex.html [ ImageOnlyFailure ]
+webkit.org/b/77568 [ Sierra HighSierra Mojave Catalina ] fast/text/locale-shaping.html [ ImageOnlyFailure ]
+webkit.org/b/77568 [ Sierra HighSierra Mojave Catalina ] fast/text/locale-shaping-complex.html [ ImageOnlyFailure ]
 
 webkit.org/b/203222 svg/wicd/rightsizing-grid.xhtml [ Pass Failure ]
 
index b7f9e4a..2e6d333 100644 (file)
@@ -1,3 +1,15 @@
+2020-02-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION(r254534): 1-3% regression on PLT
+        https://bugs.webkit.org/show_bug.cgi?id=207244
+        <rdar://problem/58821709>
+
+        Reviewed by Simon Fraser.
+
+        CTFontTransformGlyphsWithLanguage() is only acceptable on certain OSes.
+
+        * wtf/PlatformHave.h:
+
 2020-02-06  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r255910, r255970, and r255972.
index 175b6f5..bca876e 100644 (file)
 #define HAVE_CTFONTCREATEFORCHARACTERSWITHLANGUAGEANDOPTION 1
 #endif
 
-#if PLATFORM(COCOA) && !(PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101500) && !PLATFORM(WATCHOS) && !PLATFORM(APPLETV)
-#define HAVE_CTFONTTRANSFORMGLYPHSWITHLANGUAGE 1
-#endif
-
 #if PLATFORM(IOS) || PLATFORM(MACCATALYST)
 #define HAVE_ARKIT_QUICK_LOOK_PREVIEW_ITEM 1
 #endif
index cf4e057..7bfc09c 100644 (file)
 #if OS(DARWIN) && !USE(PLATFORM_REGISTERS_WITH_PROFILE) && CPU(ARM64)
 #define USE_DARWIN_REGISTER_MACROS 1
 #endif
+
+#if PLATFORM(COCOA) && !(PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101600)
+#define USE_CTFONTTRANSFORMGLYPHSWITHLANGUAGE 1
+#endif
index 16efd8a..2274a31 100644 (file)
@@ -1,3 +1,32 @@
+2020-02-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION(r254534): 1-3% regression on PLT
+        https://bugs.webkit.org/show_bug.cgi?id=207244
+        <rdar://problem/58821709>
+
+        Reviewed by Simon Fraser.
+
+        Turns out CTFontTransformGlyphsWithLanguage() is 33.7% slower than CTFontTransformGlyphs() on some OSes.
+        This patch changes the preprocessor guards to not use the function on those OSes.
+        Also, the contract of the function on the more performant OSes requires that the locale name be canonicalized,
+        so this patch implements a canonical locale cache inside LocaleMac.mm. It gets cleared when the low memory
+        warning is fired.
+
+        Marked existing tests as failing.
+
+        * page/cocoa/MemoryReleaseCocoa.mm:
+        (WebCore::platformReleaseMemory):
+        * platform/graphics/cocoa/FontCocoa.mm:
+        (WebCore::Font::applyTransforms const):
+        * platform/graphics/mac/SimpleFontDataCoreText.cpp:
+        (WebCore::Font::getCFStringAttributes const):
+        * platform/text/mac/LocaleMac.h:
+        * platform/text/mac/LocaleMac.mm:
+        (WebCore::determineLocale):
+        (WebCore::canonicalLocaleMap):
+        (WebCore::LocaleMac::canonicalLanguageIdentifierFromString):
+        (WebCore::LocaleMac::releaseMemory):
+
 2020-02-06  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: show JavaScript Worker terminated state as an internal property
index d2121e7..7cb8680 100644 (file)
@@ -30,6 +30,7 @@
 #import "GCController.h"
 #import "IOSurfacePool.h"
 #import "LayerPool.h"
+#import "LocaleMac.h"
 #import "SystemFontDatabaseCoreText.h"
 #import <notify.h>
 #import <pal/spi/ios/GraphicsServicesSPI.h>
@@ -55,6 +56,8 @@ void platformReleaseMemory(Critical)
     GSFontPurgeFontCache();
 #endif
 
+    LocaleMac::releaseMemory();
+
     for (auto& pool : LayerPool::allLayerPools())
         pool->drain();
 
index 5a3352b..93b8bb1 100644 (file)
@@ -32,6 +32,7 @@
 #import "FontCache.h"
 #import "FontCascade.h"
 #import "FontDescription.h"
+#import "LocaleMac.h"
 #import "OpenTypeCG.h"
 #import "SharedBuffer.h"
 #import <CoreText/CoreText.h>
@@ -547,7 +548,7 @@ void Font::applyTransforms(GlyphBuffer& glyphBuffer, unsigned beginningIndex, bo
 {
     // FIXME: Implement GlyphBuffer initial advance.
     CTFontTransformOptions options = (enableKerning ? kCTFontTransformApplyPositioning : 0) | (requiresShaping ? kCTFontTransformApplyShaping : 0);
-#if HAVE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
+#if USE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
     auto handler = ^(CFRange range, CGGlyph** newGlyphsPointer, CGSize** newAdvancesPointer) {
         range.location = std::min(std::max(range.location, static_cast<CFIndex>(0)), static_cast<CFIndex>(glyphBuffer.size()));
         if (range.length < 0) {
@@ -559,7 +560,7 @@ void Font::applyTransforms(GlyphBuffer& glyphBuffer, unsigned beginningIndex, bo
         *newGlyphsPointer = glyphBuffer.glyphs(beginningIndex);
         *newAdvancesPointer = glyphBuffer.advances(beginningIndex);
     };
-    CTFontTransformGlyphsWithLanguage(m_platformData.ctFont(), glyphBuffer.glyphs(beginningIndex), reinterpret_cast<CGSize*>(glyphBuffer.advances(beginningIndex)), glyphBuffer.size() - beginningIndex, options, locale.string().createCFString().get(), handler);
+    CTFontTransformGlyphsWithLanguage(m_platformData.ctFont(), glyphBuffer.glyphs(beginningIndex), reinterpret_cast<CGSize*>(glyphBuffer.advances(beginningIndex)), glyphBuffer.size() - beginningIndex, options, LocaleMac::canonicalLanguageIdentifierFromString(locale).string().createCFString().get(), handler);
 #else
     UNUSED_PARAM(locale);
     CTFontTransformGlyphs(m_platformData.ctFont(), glyphBuffer.glyphs(beginningIndex), reinterpret_cast<CGSize*>(glyphBuffer.advances(beginningIndex)), glyphBuffer.size() - beginningIndex, options);
index 8945124..97650d1 100644 (file)
@@ -37,7 +37,7 @@ RetainPtr<CFDictionaryRef> Font::getCFStringAttributes(bool enableKerning, FontO
     auto attributesDictionary = adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 4, &kCFCopyStringDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
 
     CFDictionarySetValue(attributesDictionary.get(), kCTFontAttributeName, platformData().ctFont());
-#if HAVE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
+#if USE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
     if (!locale.isEmpty())
         CFDictionarySetValue(attributesDictionary.get(), kCTLanguageAttributeName, locale.string().createCFString().get());
 #else
index be091ac..35d5c4e 100644 (file)
@@ -67,6 +67,9 @@ public:
     const Vector<String>& timeAMPMLabels() override;
 #endif
 
+    static AtomString canonicalLanguageIdentifierFromString(const AtomString&);
+    static void releaseMemory();
+
 private:
     RetainPtr<NSDateFormatter> shortDateFormatter();
     void initializeLocaleData() override;
index 2b3d03a..cdc140b 100644 (file)
 #import <Foundation/NSDateFormatter.h>
 #import <Foundation/NSLocale.h>
 #import <wtf/DateMath.h>
+#import <wtf/HashMap.h>
 #import <wtf/Language.h>
 #import <wtf/RetainPtr.h>
+#import <wtf/text/AtomStringHash.h>
 
 #if PLATFORM(IOS_FAMILY)
 #import "LocalizedDateCache.h"
@@ -62,7 +64,7 @@ static RetainPtr<NSLocale> determineLocale(const String& locale)
     if (equalIgnoringASCIICase(currentLocaleLanguage, localeLanguage))
         return currentLocale;
     // It seems initWithLocaleIdentifier accepts dash-separated locale identifier.
-     return adoptNS([[NSLocale alloc] initWithLocaleIdentifier:locale]);
+    return adoptNS([[NSLocale alloc] initWithLocaleIdentifier:locale]);
 }
 
 std::unique_ptr<Locale> Locale::create(const AtomString& locale)
@@ -276,6 +278,26 @@ const Vector<String>& LocaleMac::timeAMPMLabels()
 
 #endif
 
+using CanonicalLocaleMap = HashMap<AtomString, AtomString>;
+
+static CanonicalLocaleMap& canonicalLocaleMap()
+{
+    static NeverDestroyed<CanonicalLocaleMap> canonicalLocaleMap;
+    return canonicalLocaleMap.get();
+}
+
+AtomString LocaleMac::canonicalLanguageIdentifierFromString(const AtomString& string)
+{
+    return canonicalLocaleMap().ensure(string, [&] {
+        return [NSLocale canonicalLanguageIdentifierFromString:string];
+    }).iterator->value;
+}
+
+void LocaleMac::releaseMemory()
+{
+    canonicalLocaleMap().clear();
+}
+
 void LocaleMac::initializeLocaleData()
 {
     if (m_didInitializeNumberData)