Changeset 255988 in webkit


Ignore:
Timestamp:
Feb 6, 2020 3:31:17 PM (4 years ago)
Author:
mmaxfield@apple.com
Message:

REGRESSION(r254534): 1-3% regression on PLT
​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:
Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r255986 r255988  
     12020-02-06  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        REGRESSION(r254534): 1-3% regression on PLT
     4        https://bugs.webkit.org/show_bug.cgi?id=207244
     5        <rdar://problem/58821709>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Mark the tests as failing on certain OSes.
     10
     11        * platform/ios/TestExpectations:
     12        * platform/mac/TestExpectations:
     13
    1142020-02-06  Devin Rousso  <drousso@apple.com>
    215

  • trunk/LayoutTests/platform/ios/TestExpectations

    r255960 r255988  
    34823482
    34833483webkit.org/b/207278 imported/w3c/web-platform-tests/web-animations/timing-model/animations/finishing-an-animation.html [ Pass Failure ]
     3484
     3485# Locale-specific shaping is only enabled on certain OSes.
     3486webkit.org/b/77568 fast/text/locale-shaping.html [ ImageOnlyFailure ]
     3487webkit.org/b/77568 fast/text/locale-shaping-complex.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/platform/mac/TestExpectations

    r255978 r255988  
    19321932
    19331933# Locale-specific shaping is only enabled on certain OSes.
    1934 webkit.org/b/77568 [ Sierra HighSierra Mojave ] fast/text/locale-shaping.html [ ImageOnlyFailure ]
    1935 webkit.org/b/77568 [ Sierra HighSierra Mojave ] fast/text/locale-shaping-complex.html [ ImageOnlyFailure ]
     1934webkit.org/b/77568 [ Sierra HighSierra Mojave Catalina ] fast/text/locale-shaping.html [ ImageOnlyFailure ]
     1935webkit.org/b/77568 [ Sierra HighSierra Mojave Catalina ] fast/text/locale-shaping-complex.html [ ImageOnlyFailure ]
    19361936
    19371937webkit.org/b/203222 svg/wicd/rightsizing-grid.xhtml [ Pass Failure ]

  • trunk/Source/WTF/ChangeLog

    r255973 r255988  
     12020-02-06  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        REGRESSION(r254534): 1-3% regression on PLT
     4        https://bugs.webkit.org/show_bug.cgi?id=207244
     5        <rdar://problem/58821709>
     6
     7        Reviewed by Simon Fraser.
     8
     9        CTFontTransformGlyphsWithLanguage() is only acceptable on certain OSes.
     10
     11        * wtf/PlatformHave.h:
     12
    1132020-02-06  Commit Queue  <commit-queue@webkit.org>
    214

  • trunk/Source/WTF/wtf/PlatformHave.h

    r255973 r255988  
    463463#endif
    464464
    465 #if PLATFORM(COCOA) && !(PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101500) && !PLATFORM(WATCHOS) && !PLATFORM(APPLETV)
    466 #define HAVE_CTFONTTRANSFORMGLYPHSWITHLANGUAGE 1
    467 #endif
    468 
    469465#if PLATFORM(IOS) || PLATFORM(MACCATALYST)
    470466#define HAVE_ARKIT_QUICK_LOOK_PREVIEW_ITEM 1

  • trunk/Source/WTF/wtf/PlatformUse.h

    r255216 r255988  
    312312#define USE_DARWIN_REGISTER_MACROS 1
    313313#endif
     314
     315#if PLATFORM(COCOA) && !(PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101600)
     316#define USE_CTFONTTRANSFORMGLYPHSWITHLANGUAGE 1
     317#endif

  • trunk/Source/WebCore/ChangeLog

    r255986 r255988  
     12020-02-06  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        REGRESSION(r254534): 1-3% regression on PLT
     4        https://bugs.webkit.org/show_bug.cgi?id=207244
     5        <rdar://problem/58821709>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Turns out CTFontTransformGlyphsWithLanguage() is 33.7% slower than CTFontTransformGlyphs() on some OSes.
     10        This patch changes the preprocessor guards to not use the function on those OSes.
     11        Also, the contract of the function on the more performant OSes requires that the locale name be canonicalized,
     12        so this patch implements a canonical locale cache inside LocaleMac.mm. It gets cleared when the low memory
     13        warning is fired.
     14
     15        Marked existing tests as failing.
     16
     17        * page/cocoa/MemoryReleaseCocoa.mm:
     18        (WebCore::platformReleaseMemory):
     19        * platform/graphics/cocoa/FontCocoa.mm:
     20        (WebCore::Font::applyTransforms const):
     21        * platform/graphics/mac/SimpleFontDataCoreText.cpp:
     22        (WebCore::Font::getCFStringAttributes const):
     23        * platform/text/mac/LocaleMac.h:
     24        * platform/text/mac/LocaleMac.mm:
     25        (WebCore::determineLocale):
     26        (WebCore::canonicalLocaleMap):
     27        (WebCore::LocaleMac::canonicalLanguageIdentifierFromString):
     28        (WebCore::LocaleMac::releaseMemory):
     29
    1302020-02-06  Devin Rousso  <drousso@apple.com>
    231

  • trunk/Source/WebCore/page/cocoa/MemoryReleaseCocoa.mm

    r247465 r255988  
    3131#import "IOSurfacePool.h"
    3232#import "LayerPool.h"
     33#import "LocaleMac.h"
    3334#import "SystemFontDatabaseCoreText.h"
    3435#import <notify.h>
    … …  
    5556    GSFontPurgeFontCache();
    5657#endif
     58
     59    LocaleMac::releaseMemory();
    5760
    5861    for (auto& pool : LayerPool::allLayerPools())

  • trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm

    r254534 r255988  
    3333#import "FontCascade.h"
    3434#import "FontDescription.h"
     35#import "LocaleMac.h"
    3536#import "OpenTypeCG.h"
    3637#import "SharedBuffer.h"
    … …  
    548549    // FIXME: Implement GlyphBuffer initial advance.
    549550    CTFontTransformOptions options = (enableKerning ? kCTFontTransformApplyPositioning : 0) | (requiresShaping ? kCTFontTransformApplyShaping : 0);
    550 #if HAVE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
     551#if USE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
    551552    auto handler = ^(CFRange range, CGGlyph** newGlyphsPointer, CGSize** newAdvancesPointer) {
    552553        range.location = std::min(std::max(range.location, static_cast<CFIndex>(0)), static_cast<CFIndex>(glyphBuffer.size()));
    … …  
    560561        *newAdvancesPointer = glyphBuffer.advances(beginningIndex);
    561562    };
    562     CTFontTransformGlyphsWithLanguage(m_platformData.ctFont(), glyphBuffer.glyphs(beginningIndex), reinterpret_cast<CGSize*>(glyphBuffer.advances(beginningIndex)), glyphBuffer.size() - beginningIndex, options, locale.string().createCFString().get(), handler);
     563    CTFontTransformGlyphsWithLanguage(m_platformData.ctFont(), glyphBuffer.glyphs(beginningIndex), reinterpret_cast<CGSize*>(glyphBuffer.advances(beginningIndex)), glyphBuffer.size() - beginningIndex, options, LocaleMac::canonicalLanguageIdentifierFromString(locale).string().createCFString().get(), handler);
    563564#else
    564565    UNUSED_PARAM(locale);

  • trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp

    r254534 r255988  
    3838
    3939    CFDictionarySetValue(attributesDictionary.get(), kCTFontAttributeName, platformData().ctFont());
    40 #if HAVE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
     40#if USE(CTFONTTRANSFORMGLYPHSWITHLANGUAGE)
    4141    if (!locale.isEmpty())
    4242        CFDictionarySetValue(attributesDictionary.get(), kCTLanguageAttributeName, locale.string().createCFString().get());

  • trunk/Source/WebCore/platform/text/mac/LocaleMac.h

    r244664 r255988  
    6868#endif
    6969
     70    static AtomString canonicalLanguageIdentifierFromString(const AtomString&);
     71    static void releaseMemory();
     72
    7073private:
    7174    RetainPtr<NSDateFormatter> shortDateFormatter();

  • trunk/Source/WebCore/platform/text/mac/LocaleMac.mm

    r248846 r255988  
    3636#import <Foundation/NSLocale.h>
    3737#import <wtf/DateMath.h>
     38#import <wtf/HashMap.h>
    3839#import <wtf/Language.h>
    3940#import <wtf/RetainPtr.h>
     41#import <wtf/text/AtomStringHash.h>
    4042
    4143#if PLATFORM(IOS_FAMILY)
    … …  
    6365        return currentLocale;
    6466    // It seems initWithLocaleIdentifier accepts dash-separated locale identifier.
    65      return adoptNS([[NSLocale alloc] initWithLocaleIdentifier:locale]);
     67    return adoptNS([[NSLocale alloc] initWithLocaleIdentifier:locale]);
    6668}
    6769
    … …  
    277279#endif
    278280
     281using CanonicalLocaleMap = HashMap<AtomString, AtomString>;
     282
     283static CanonicalLocaleMap& canonicalLocaleMap()
     284{
     285    static NeverDestroyed<CanonicalLocaleMap> canonicalLocaleMap;
     286    return canonicalLocaleMap.get();
     287}
     288
     289AtomString LocaleMac::canonicalLanguageIdentifierFromString(const AtomString& string)
     290{
     291    return canonicalLocaleMap().ensure(string, [&] {
     292        return [NSLocale canonicalLanguageIdentifierFromString:string];
     293    }).iterator->value;
     294}
     295
     296void LocaleMac::releaseMemory()
     297{
     298    canonicalLocaleMap().clear();
     299}
     300
    279301void LocaleMac::initializeLocaleData()
    280302{
Note: See TracChangeset for help on using the changeset viewer.