Attachment 339159 Details for Bug 184720 – Patch

[patch] Patch

bug-184720-20180430162044.patch (text/plain), 4.72 KB, created by Myles C. Maxfield on 2018-04-30 16:20:45 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Myles C. Maxfield

Created: 2018-04-30 16:20:45 PDT

Size: 4.72 KB

patch

obsolete

>Subversion Revision: 231125
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 005ee1e0d97e91fa7119eb1d5bf0bbe6cac213d6..40df8442f7b7f40c363889d3e5a55c365f4fe830 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-04-30  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        Improve the performance of FontCascadeDescription's effectiveFamilies
>+        https://bugs.webkit.org/show_bug.cgi?id=184720
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The page that had the performance problem renders many different Chinese characters in system-ui
>+        with roughly few number of individual fonts. It turns out we were calling into the system-ui
>+        machinery for each character in order to opportunistically start loading data URLs (see also:
>+        https://bugs.webkit.org/show_bug.cgi?id=175845). These data URLS are never for the system font,
>+        so we don't need to invoke the system-ui machinery at all.
>+
>+        This patch makes a 92x performance improvement on the associated performance test. This test is
>+        designed to test Chinese text rendered with system-ui.
>+
>+        Performance test: Layout/system-ui.html
>+
>+        * platform/graphics/FontCascadeFonts.cpp:
>+        (WebCore::opportunisticallyStartFontDataURLLoading):
>+
> 2018-04-27  Simon Fraser  <simon.fraser@apple.com>
> 
>         Implement color-filter for text stroke
>diff --git a/Source/WebCore/platform/graphics/FontCascadeFonts.cpp b/Source/WebCore/platform/graphics/FontCascadeFonts.cpp
>index cf0cf780fdff59d71ebe29499572b7e4934df769..5dccc19445c4f0e9da3b7eccc4ef6383c11f615f 100644
>--- a/Source/WebCore/platform/graphics/FontCascadeFonts.cpp
>+++ b/Source/WebCore/platform/graphics/FontCascadeFonts.cpp
>@@ -389,14 +389,8 @@ static void opportunisticallyStartFontDataURLLoading(const FontCascadeDescriptio
>     // asynchronous, and this code doesn't actually fix the race - it just makes it more likely for the two fonts to tie in the race.
>     if (!fontSelector)
>         return;
>-    for (unsigned i = 0; i < description.effectiveFamilyCount(); ++i) {
>-        auto visitor = WTF::makeVisitor([&](const AtomicString& family) {
>-            fontSelector->opportunisticallyStartFontDataURLLoading(description, family);
>-        }, [&](const FontFamilyPlatformSpecification&) {
>-        });
>-        const auto& currentFamily = description.effectiveFamilyAt(i);
>-        WTF::visit(visitor, currentFamily);
>-    }
>+    for (unsigned i = 0; i < description.familyCount(); ++i)
>+        fontSelector->opportunisticallyStartFontDataURLLoading(description, description.familyAt(i));
> }
> 
> GlyphData FontCascadeFonts::glyphDataForVariant(UChar32 character, const FontCascadeDescription& description, FontVariant variant, unsigned fallbackIndex)
>diff --git a/PerformanceTests/ChangeLog b/PerformanceTests/ChangeLog
>index f88d796c053d2fe568c1292cd7111e6e735ee05c..f6f454e81de61c38a11adc2992e9be6ade23056a 100644
>--- a/PerformanceTests/ChangeLog
>+++ b/PerformanceTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-04-30  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        Improve the performance of FontCascadeDescription's effectiveFamilies
>+        https://bugs.webkit.org/show_bug.cgi?id=184720
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This performance test calls SystemFontDatabase::systemFontCascadeList() around 2,000,000 times (before
>+        this patch is applied), which is roughly equivalent to the page we found the performance problem on.
>+        The calling pattern is roughly equivalent in this test.
>+
>+        * Layout/system-ui.html: Added.
>+
> 2018-03-08  Antti Koivisto  <antti@apple.com>
> 
>         Update StyleBench version number in page title to 0.3
>diff --git a/PerformanceTests/Layout/system-ui.html b/PerformanceTests/Layout/system-ui.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..b52f4040f1787d527bf341f73858dd750216844d
>--- /dev/null
>+++ b/PerformanceTests/Layout/system-ui.html
>@@ -0,0 +1,36 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<script src="../resources/runner.js"></script>
>+</head>
>+<body>
>+<div id="target" style="width: 300px; display: none; font: 42px 'system-ui', '-apple-system';" lang="zh-CN"></div>
>+<script>
>+var target = document.getElementById("target");
>+var style = target.style;
>+
>+var s = "";
>+var length = 10000;
>+var startCode = 0x4E00;
>+for (var i = 0; i < length; ++i) {
>+    s = s + String.fromCharCode(i + startCode);
>+}
>+
>+
>+function test() {
>+    if (window.internals)
>+        window.internals.invalidateFontCache();
>+
>+    style.display = "block";
>+    target.offsetLeft;
>+    target.textContent = s;
>+    target.offsetLeft;
>+    target.textContent = "";
>+    style.display = "none";
>+}
>+
>+PerfTestRunner.measureRunsPerSecond({ run: test });
>+</script>
>+</body>
>+</html>

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 184720: 338168 | 338208 | 339159