Bug 207374 – [Cocoa] Slightly improve performance of Font::getCFStringAttributes()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

[Cocoa] Slightly improve performance of Font::getCFStringAttributes()

[Myles C. Maxfield]

Created attachment 390046 [details]
Patch

[Myles C. Maxfield]

Created attachment 390048 [details]
Patch

[Darin Adler]

Comment on attachment 390048 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390048&action=review

This looks good. Like I said earlier, if this is *really* hot then we should go even further with some kind of caching/memoizing so we don’t create a dictionary at all in most cases.

> Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:38
> +    CFTypeRef keys[5];
> +    CFTypeRef values[5];

Easy mistake would be to add a new case below and not update the array sizes here. Would be nice if we could find a way to assert or static_assert or something to make it trivial to catch the mistake either at compile time or runtime.

> Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:59
> +    static CTParagraphStyleRef paragraphStyle = nullptr;
> +    if (!paragraphStyle) {
> +        paragraphStyle = CTParagraphStyleCreate(nullptr, 0);
> +        CTParagraphStyleSetCompositionLanguage(paragraphStyle, kCTCompositionLanguageNone);
> +    }

This can be written something like this:

    static CTParagraphStyleRef paragraphStyle = [] () {
        auto style = CTParagraphStyleCreate(nullptr, 0);
        CGParagraphStyleSetCompositionLanguage(style, kCTCompositionLanguageNone);
        return style;
     }();

I think it’s slightly better coding style. Could even put this into a separate function for clarity.

[Myles C. Maxfield]

Created attachment 390103 [details]
Patch for committing

[WebKit Commit Bot]

Comment on attachment 390103 [details]
Patch for committing

Clearing flags on attachment: 390103

Committed r256037: <https://trac.webkit.org/changeset/256037>

[Radar WebKit Bug Importer]

<rdar://problem/59269074>