Bug 161119 – [Cocoa] Improve performance of glyph advance metrics gathering

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

[Myles C. Maxfield]

[Cocoa] Improve performance of glyph advance metrics gathering

[Myles C. Maxfield]

Created attachment 286808 [details]
WIP

[Myles C. Maxfield]

Created attachment 286823 [details]
WIP

[Myles C. Maxfield]

Created attachment 286828 [details]
WIP

[WebKit Commit Bot]

Attachment 286828 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

This patch makes PLT worse by 0.46%.

[Myles C. Maxfield]

Created attachment 287002 [details]
Perf Measurements

Current CG and CT APIs for measuring width allow you to specify multiple glyphs at once. I measured the runtime of measuring increasingly large numbers of glyphs, and you can see that the relationship is linear with a y-intercept of 0.

This means that calling the function twice with half the number of glyphs has the same runtime cost as running the function once with the full set of glyphs. Therefore, batching the glyphs up into fewer calls has no runtime gain.

[Myles C. Maxfield]

Created attachment 287003 [details]
Perf measurements - excluding glyph counts of > 256

If you exclude the data points that we will obviously never use (because they're too big) you can see the y-intercept isn't actually 0.

[Myles C. Maxfield]

Created attachment 287008 [details]
Y-Intercept zoom

You can see that the y intercept is about 8 times bigger than the slope, which means we want at most 8 unused measurements per call. Any more unused measurements, and it would be cheaper just to make a new call. (However, this logic assumes that all the 8 unused measurements can be gathered in a single call.)

This data seems to indicate that we would still benefit from aggregation, but only if the group size is on the order of 8 glyphs.

[Myles C. Maxfield]

Created attachment 287012 [details]
Get glyphs from characters

For fun, here is the similar graph for getting glyphs from characters.

[Myles C. Maxfield]

Created attachment 287013 [details]
Y intercept for getting glyphs from characters

For getting glyphs from characters, the Y-intercept is about 5 times as big as the slope

[Myles C. Maxfield]

This line of investigation needs to consider:
1) The hit rate of the current cache.
2) The number of glyphs / advances which are gathered but never used

[Myles C. Maxfield]

Only 6.55% of the character -> glyph mappings we populate our GlyphPages with are ever actually read.

This seems to indicate the glyph page size is way too big.

[Myles C. Maxfield]

(In reply to comment #12)
> Only 6.55% of the character -> glyph mappings we populate our GlyphPages
> with are ever actually read.
> 
> This seems to indicate the glyph page size is way too big.

Decreasing the glyph page size to 16 boosts this up to 25%.

Decreasing the glyph page size to 8 boosts this up to 36%.

(And, of course, decreasing the glyph page size to 1 boosts this up to 100%.)

[Myles C. Maxfield]

(In reply to comment #13)
> (In reply to comment #12)
> > Only 6.55% of the character -> glyph mappings we populate our GlyphPages
> > with are ever actually read.
> > 
> > This seems to indicate the glyph page size is way too big.
> 
> Decreasing the glyph page size to 16 boosts this up to 25%.
> 
> Decreasing the glyph page size to 8 boosts this up to 36%.
> 
> (And, of course, decreasing the glyph page size to 1 boosts this up to 100%.)

Similarly,

With the current glyph page size, we populate 715 pages.

Decreasing the glyph page size to 16 makes us create 2879 pages.

Decreasing the glyph page size to 8 makes us create 4181 pages.

[Myles C. Maxfield]

Decreasing the glyph page size from 256 to 16 causes:

2164 more glyph pages to be created (and therefore 2164 more calls to map characters to glyphs)

but, we then avoid creating 136976 unread mappings.

Given that one call is roughly the cost of creating an additional 5 mappings, this should definitely be a win.

This is actually a memory win too, because we'll be creating fewer glyph pages to remember.

[Myles C. Maxfield]

Created attachment 287047 [details]
WIP

[Myles C. Maxfield]

Created attachment 287050 [details]
WIP

[Myles C. Maxfield]

Created attachment 287086 [details]
WIP

[Myles C. Maxfield]

Created attachment 287117 [details]
WIP

[Myles C. Maxfield]

Created attachment 287122 [details]
WIP

[Myles C. Maxfield]

This latest patch causes PLT to improve.

[Myles C. Maxfield]

Created attachment 287214 [details]
WIP

[Myles C. Maxfield]

+css3/font-variant-all.html		reference images diff (0.47%)	image	pass	history
+fast/css/font-face-multiple-faces.html	expected actual diff pretty diff	images diff (3.28%)	text image+text	pass	history
+fast/css/font-face-synthetic-bold-italic.html	expected actual diff pretty diff	images diff (1.36%)	text image+text	pass	history
+fast/css/font-face-weight-matching.html	expected actual diff pretty diff	images diff (1.11%)	text image+text	pass	history
+fast/text/arabic-zwj-and-zwnj.html		reference images diff (0.04%)	image	pass	history
+fast/text/atsui-multiple-renderers.html	expected actual diff pretty diff	images diff (2.82%)	text image+text	pass	history
+fast/text/complex-initial-advance.html		reference images diff (0.05%)	image	pass	history
+fast/text/complex-synthetic-bold-space-width.html	expected actual diff pretty diff	images diff (0.2%)	text image+text	pass	history
+fast/text/drawBidiText.html	expected actual diff pretty diff	images diff (1.77%)	text image+text	pass	history
+fast/text/emoji-num-glyphs.html			timeout fail	fail	history
+fast/text/font-size-zero.html	expected actual diff pretty diff		text	pass	history
+fast/text/font-weights-zh.html	expected actual diff pretty diff	images diff (0.14%)	text image+text	pass	history
+fast/text/format-control.html	expected actual diff pretty diff	images diff (1.42%)	text image+text	pass	history
+fast/text/hyphenate-avoid-orphaned-word.html	expected actual diff pretty diff	images	text missing		history
+fast/text/hyphenate-character.html	expected actual diff pretty diff	images diff (5.8%)	text image+text	pass	history
+fast/text/hyphenate-first-word.html	expected actual diff pretty diff	images diff (0.29%)	text image+text	pass	history
+fast/text/hyphenate-limit-lines.html	expected actual diff pretty diff	images diff (4.04%)	text image+text	pass	history
+fast/text/hyphenate-locale.html	expected actual diff pretty diff	images diff (0.86%)	text image+text	pass	history
+fast/text/hyphens.html	expected actual diff pretty diff	images diff (5.41%)	text image+text	pass	history
+fast/text/line-initial-and-final-swashes.html	expected actual diff pretty diff	images diff (3.22%)	text image+text	pass	history
+fast/text/multiple-feature-properties.html		reference images diff (0.09%)	image	pass	history
+fast/text/narrow-non-breaking-space.html		reference images diff (0.01%)	image	pass	history
+fast/text/wide-zero-width-space.html	expected actual diff pretty diff	images diff (1.09%)	text image+text	pass	history
+fast/text/zero-font-size.html	expected actual diff pretty diff	images diff (0.82%)	text image+text	pass	history
+fast/text/zero-sized-fonts.html	expected actual diff pretty diff		text	pass	history
+fast/writing-mode/broken-ideographic-font.html	expected actual diff pretty diff	images diff (0.5%)	text image+text	pass	history
+fast/writing-mode/japanese-rl-text-with-broken-font.html	expected actual diff pretty diff	images diff (5.95%)	text image+text	pass	history
+fonts/cursive.html	expected actual diff pretty diff	images diff (0.73%)	text image+text	pass	history
+fonts/fantasy.html	expected actual diff pretty diff	images diff (0.79%)	text image+text	pass	history
+fonts/monospace.html	expected actual diff pretty diff	images diff (7.21%)	text image+text	pass	history
+imported/blink/fast/text/international/text-shaping-arabic.html		reference images diff (0.01%)	image	pass	history
+imported/w3c/web-platform-tests/html/dom/elements/global-attributes/dir_auto-textarea-mixed.html		reference images diff (0.19%)	image	pass	history
+imported/w3c/web-platform-tests/html/dom/elements/global-attributes/dir_auto-textarea-script-mixed.html		reference images diff (0.19%)	image	pass	history
+mathml/opentype/munderover-layout-resize.html		reference images diff (0.29%)	image	pass	history
+platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html	expected actual diff pretty diff	images diff (4.66%)	text image+text	pass	history
+svg/W3C-SVG-1.1-SE/filters-image-03-f.svg	expected actual diff pretty diff	images diff (0.43%)	text image+text	pass	history
+svg/W3C-SVG-1.1-SE/paths-dom-02-f.svg	expected actual diff pretty diff	images diff (0.26%)	text image+text	pass	history
+svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg	expected actual diff pretty diff	images diff (0.24%)	text image+text	pass	history
+svg/W3C-SVG-1.1-SE/struct-use-14-f.svg	expected actual diff pretty diff	images diff (0.26%)	text image+text	pass	history
+svg/W3C-SVG-1.1-SE/svgdom-over-01-f.svg	expected actual diff pretty diff	images diff (1.42%)	text image+text	pass	history
+svg/W3C-SVG-1.1/fonts-glyph-02-t.svg	expected actual diff pretty diff	images diff (2.94%)	text image+text	pass	history
+svg/W3C-SVG-1.1/interact-cursor-01-f.svg	expected actual diff pretty diff	images diff (1.89%)	text image+text	pass	history
+svg/custom/glyph-selection-arabic-forms.svg	expected actual diff pretty diff	images diff (0.2%)	text image+text	pass	history
+svg/custom/svg-fonts-fallback.xhtml	expected actual diff pretty diff	images diff (0.6%)	text image+text	pass	history
+svg/text/font-size-below-point-five.svg	expected actual diff pretty diff	images diff (0.37%)	text image+text	pass	history

[Myles C. Maxfield]

Created attachment 287922 [details]
WIP

[Myles C. Maxfield]

Created attachment 288169 [details]
Testing CoreText

[Myles C. Maxfield]

Created attachment 288183 [details]
Using CoreText measurements

[Myles C. Maxfield]

Created attachment 288210 [details]
platformWidthForGlyph

[Myles C. Maxfield]

Created attachment 288223 [details]
GlyphPage size

[WebKit Commit Bot]

Attachment 288223 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 288223 [details]
GlyphPage size

Attachment 288223 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2030099

New failing tests:
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
imported/blink/fast/text/international/text-shaping-arabic.html
fast/text/multiple-feature-properties.html
fast/text/arabic-zwj-and-zwnj.html
svg/custom/svg-fonts-fallback.xhtml
css3/font-variant-all.html
fast/text/complex-initial-advance.html
svg/custom/glyph-selection-arabic-forms.svg
mathml/presentation/non-bmp-operators-stretching.html
platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html

[Build Bot]

Created attachment 288228 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 288223 [details]
GlyphPage size

Attachment 288223 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2030109

New failing tests:
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
imported/blink/fast/text/international/text-shaping-arabic.html
fast/text/multiple-feature-properties.html
fast/text/arabic-zwj-and-zwnj.html
svg/custom/svg-fonts-fallback.xhtml
css3/font-variant-all.html
fast/text/complex-initial-advance.html
svg/custom/glyph-selection-arabic-forms.svg
mathml/presentation/non-bmp-operators-stretching.html
platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html

[Build Bot]

Created attachment 288230 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 288223 [details]
GlyphPage size

Attachment 288223 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2030105

New failing tests:
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
imported/blink/fast/text/international/text-shaping-arabic.html
fast/text/multiple-feature-properties.html
fast/text/arabic-zwj-and-zwnj.html
svg/custom/svg-fonts-fallback.xhtml
css3/font-variant-all.html
fast/text/complex-initial-advance.html
svg/custom/glyph-selection-arabic-forms.svg
mathml/presentation/non-bmp-operators-stretching.html
platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html

[Build Bot]

Created attachment 288231 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Myles C. Maxfield]

Created attachment 288282 [details]
GlyphPage size

[WebKit Commit Bot]

Attachment 288282 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 288282 [details]
GlyphPage size

Attachment 288282 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2035570

New failing tests:
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
imported/blink/fast/text/international/text-shaping-arabic.html
mathml/presentation/non-bmp-operators-stretching.html
svg/custom/svg-fonts-fallback.xhtml
fast/text/arabic-zwj-and-zwnj.html
fast/text/multiple-feature-properties.html
css3/font-variant-all.html
fast/text/complex-initial-advance.html
svg/custom/glyph-selection-arabic-forms.svg
platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html

[Build Bot]

Created attachment 288293 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 288282 [details]
GlyphPage size

Attachment 288282 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2035642

New failing tests:
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
imported/blink/fast/text/international/text-shaping-arabic.html
fast/text/multiple-feature-properties.html
fast/text/arabic-zwj-and-zwnj.html
svg/custom/svg-fonts-fallback.xhtml
css3/font-variant-all.html
fast/text/complex-initial-advance.html
svg/custom/glyph-selection-arabic-forms.svg
mathml/presentation/non-bmp-operators-stretching.html
platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html

[Build Bot]

Created attachment 288295 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 288282 [details]
GlyphPage size

Attachment 288282 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2035666

New failing tests:
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
imported/blink/fast/text/international/text-shaping-arabic.html
fast/text/multiple-feature-properties.html
fast/text/arabic-zwj-and-zwnj.html
svg/custom/svg-fonts-fallback.xhtml
css3/font-variant-all.html
fast/text/complex-initial-advance.html
svg/custom/glyph-selection-arabic-forms.svg
mathml/presentation/non-bmp-operators-stretching.html
platform/mac/fast/text/international/Geeza-Pro-vertical-metrics-adjustment.html

[Build Bot]

Created attachment 288297 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 288282 [details]
GlyphPage size

Attachment 288282 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2035646

New failing tests:
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
fast/text/arabic-zwj-and-zwnj.html
fast/text/international/arabic-tatweel-join.html
svg/custom/glyph-selection-arabic-forms.svg
fast/text/complex-initial-advance.html
mathml/presentation/non-bmp-operators-stretching.html

[Build Bot]

Created attachment 288299 [details]
Archive of layout-test-results from ews123 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.5

[Myles C. Maxfield]

Created attachment 288319 [details]
GlyphPage size

[WebKit Commit Bot]

Attachment 288319 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 288330 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 288330 [details]
Patch

0' / GlyphPage::size everywhere. That should be wrapped in pageIndexForGlyph() or something.

[Myles C. Maxfield]

Created attachment 288343 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 288343 [details]
Patch

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

> Source/WebCore/platform/graphics/Font.cpp:155
> +    if (pageNumber >= 0x60 && pageNumber < 0x70 && font.shouldNotBeUsedForArabic())

Isn't his making assumptions about page size? Shouldn't you use pageNumberForCharacter() here?

Also would prefer pageIndexForCharacter everywhere.

> Source/WebCore/platform/graphics/Font.cpp:186
> +        } else if (pageNumber == 1) {
> +            // Control characters must not render at all.
> +            for (unsigned i = 0; i < 0x10; ++i)
> +                buffer[i] = zeroWidthSpace;
> +        } else if (start == (0x7F & ~(GlyphPage::size - 1))) {
> +            buffer[0x7F - start] = zeroWidthSpace;
> +        } else if (pageNumber >= 8 && pageNumber < 10) {
> +            for (unsigned i = 0; i < GlyphPage::size; ++i)
> +                buffer[i] = zeroWidthSpace;
> +        } else if (pageNumber == 10) {
> +            buffer[softHyphen - start] = zeroWidthSpace;

I think you could write all this code in terms of glyphIndex % pageSize. Or hide that in a function that would allow you to support a larger first page in future.

> Source/WebCore/platform/graphics/FontCascadeFonts.cpp:431
> +    const unsigned pageNumber = GlyphPage::pageNumberForCharacter(c);

pageIndex

> Source/WebCore/platform/graphics/GlyphMetricsMap.h:57
> +        static const size_t size = 16;

Should you static assert the this is the same as GlyphPage::size?

> Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:639
> +    CGFontRenderingStyle style = kCGFontRenderingStyleAntialiasing | kCGFontRenderingStyleSubpixelPositioning | kCGFontRenderingStyleSubpixelQuantization | kCGFontAntialiasingStyleUnfiltered;
> +
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 100000)
> +    if (platformData().size()) {
> +        CTFontOrientation orientation = horizontal || m_isBrokenIdeographFallback ? kCTFontOrientationHorizontal : kCTFontOrientationVertical;
> +        // FIXME: Remove this special-casing when <rdar://problem/28197291> is fixed.
> +        if (CTFontIsAppleColorEmoji(m_platformData.ctFont()))
> +            CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), orientation, &glyph, &advance, 1);
> +        else
> +            CTFontGetUnsummedAdvancesForGlyphsAndStyle(m_platformData.ctFont(), orientation, style, &glyph, &advance, 1);
> +    }
> +
> +#else

This seems unrelated.

[Myles C. Maxfield]

Comment on attachment 288343 [details]
Patch

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

>> Source/WebCore/platform/graphics/GlyphMetricsMap.h:57
>> +        static const size_t size = 16;
> 
> Should you static assert the this is the same as GlyphPage::size?

It doesn't have to be the same. Reducing this size is a memory win (because most of the entries in GlyphMetricsPages are unread).

>> Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:639
>> +#else
> 
> This seems unrelated.

This is part of the performance improvements.

[Myles C. Maxfield]

Created attachment 288365 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 288365 [details]
Patch

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

> Source/WebCore/platform/graphics/GlyphPage.h:81
> +    static bool pageNumberIsUsedForArabic(unsigned pageNumber) { return pageNumber >= 0x60 && pageNumber < 0x70; }

Still have assumptions about page size here.

[Myles C. Maxfield]

Created attachment 288368 [details]
Patch for committing

[WebKit Commit Bot]

Comment on attachment 288368 [details]
Patch for committing

Rejecting attachment 288368 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 288368, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/2037790

[Myles C. Maxfield]

Created attachment 288392 [details]
Patch for committing

[Myles C. Maxfield]

Created attachment 288393 [details]
Patch for committing

[WebKit Commit Bot]

Comment on attachment 288393 [details]
Patch for committing

Clearing flags on attachment: 288393

Committed r205703: <http://trac.webkit.org/changeset/205703>

[Tim Nguyen (:ntim)]

*** Bug 152293 has been marked as a duplicate of this bug. ***