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.
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.
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.
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.
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
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
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.
(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%.)
(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.
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.
+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
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.
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
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
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
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.
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
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
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
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
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.
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.
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.
2016-08-23 17:07 PDT, Myles C. Maxfield
2016-08-23 19:36 PDT, Myles C. Maxfield
2016-08-23 20:28 PDT, Myles C. Maxfield
2016-08-25 13:41 PDT, Myles C. Maxfield
2016-08-25 13:43 PDT, Myles C. Maxfield
2016-08-25 13:54 PDT, Myles C. Maxfield
2016-08-25 14:02 PDT, Myles C. Maxfield
2016-08-25 14:06 PDT, Myles C. Maxfield
2016-08-25 17:05 PDT, Myles C. Maxfield
2016-08-25 17:23 PDT, Myles C. Maxfield
2016-08-26 02:20 PDT, Myles C. Maxfield
2016-08-26 10:21 PDT, Myles C. Maxfield
2016-08-26 11:00 PDT, Myles C. Maxfield
2016-08-27 14:42 PDT, Myles C. Maxfield
2016-09-04 15:02 PDT, Myles C. Maxfield
2016-09-07 12:26 PDT, Myles C. Maxfield
2016-09-07 14:08 PDT, Myles C. Maxfield
2016-09-07 17:14 PDT, Myles C. Maxfield
2016-09-07 19:06 PDT, Myles C. Maxfield
2016-09-07 20:07 PDT, Build Bot
2016-09-07 20:09 PDT, Build Bot
2016-09-07 20:11 PDT, Build Bot
2016-09-08 11:03 PDT, Myles C. Maxfield
2016-09-08 11:57 PDT, Build Bot
2016-09-08 12:04 PDT, Build Bot
2016-09-08 12:09 PDT, Build Bot
2016-09-08 12:15 PDT, Build Bot
2016-09-08 14:01 PDT, Myles C. Maxfield
2016-09-08 14:54 PDT, Myles C. Maxfield
2016-09-08 15:43 PDT, Myles C. Maxfield
2016-09-08 17:21 PDT, Myles C. Maxfield
2016-09-08 17:39 PDT, Myles C. Maxfield
2016-09-09 01:00 PDT, Myles C. Maxfield
2016-09-09 01:02 PDT, Myles C. Maxfield