1765093 - SVG containing large amounts of text freezes the tab

Status: VERIFIED FIXED

(Core :: SVG, defect, P3)

Description

[alexitx]

Attachment: shields.io badge for py7zr license containing the LGPLv2 text

Steps to reproduce:

Attempt to load an SVG with >20000 characters of text in a <text> element so the resulting image has very large size.

The readme page of a GitHub repository has an SVG image badge which mistakenly contains the whole LGPLv2 license text. This can pontentially be abused to hinder the loading speed of a page with such SVG.

Actual results:

The tab becomes completely unresponsive and stays in a loading state for a while before eventually loading the element. Switching tabs may cause the SVG to reload or the element to get redrawn which freezes the tab again.

Expected results:

The SVG element should be drawn quickly while overflowing outside the page, or should not get loaded at all for performance reasons.

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Graphics: WebRender' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Mayank Bansal]

https://share.firefox.dev/37XJKfu

Comment 3

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:jfkthame, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Jonathan Kew [:jfkthame]]

This seems to be triggered by the use of the textLength attribute on the huge <text> element; without this, the performance is vastly better.

The problem is that when textLength is present, the SVGTextFrame will generate a separate TextRenderedRun for each individual character, because we pass true for the aRunPerGlyph parameter to ResolvePositions here. These TextRenderedRun instances will all refer to the same textframe and textrun, but the SVG drawing code processes them one at a time in order to be able to do per-glyph adjustments.

The largest cost here actually comes from the TextRenderedRunIterator calling GetBaselinePosition separately for each of those 20K+ runs; GetBaselinePosition in turn calls gfxTextRun::MeasureText, which iterates over the entire 20K-char textrun in order to accumulate the metrics. So in effect, the code is quadratic in the length of the <text> element.

However, GetBaselinePosition doesn't actually need all the metrics that MeasureText returns; it only cares about the ascent and descent. So we can make a big improvement here by providing a cut-down API on gfxTextRun that just walks the glyph runs and accumulates ascent + descent metrics, but doesn't measure the actual glyph widths and extents.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1765093 - patch 1 - Avoid measuring entire textrun in SVGTextFrame's GetBaselinePosition function. r=#layout-reviewers

This aims to prevent an O(n^2) performance situation when per-glyph
positioning is in use.

This reduces the time to load the testcase here from around 70s to 17s
in my local build. Still not great, but at least it's less of a disaster.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1765093 - patch 2 - Make ShrinkToLigatureBoundaries return whether any adjustment was made, so callers can skip redundant calls to partial-ligature code. r=#gfx-reviewers

This allows a number of callers to skip calling partial-ligature functions
altogether if the ranges are known to be empty, which should be a minor win.

Depends on D145631

Comment 7

[Jonathan Kew [:jfkthame]]

For comparison, this is a profile of loading attachment 9272641 [details] in a local m-c build:
https://share.firefox.dev/3w8toJn

With the above patches:
https://share.firefox.dev/3yhxRMS

Comment 8

[Jonathan Kew [:jfkthame]]

Looking at the code and experimenting with testcases, I noticed that GetAscentAndDescentInAppUnits (used by APIs such as getExtentOfChar) has the same issue as GetBaselinePosition, calling the expensive gfxTextRun::MeasureText when all it wants are the line-height metrics, so it can benefit from the same optimization.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1765093 - patch 3 - Also use the new gfxTextRun::GetLineHeightMetrics to optimize GetAscentAndDescentInAppUnits. r=#layout-reviewers

This accelerates web-exposed SVG APIs such as getExtentOfChar() by avoiding the expensive
MeasureText() call when we're just getting the line-height metrics.

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: testcase that exercises the codepaths here

For reference, this is a test file I've been using to profile locally. With the current patches here, my local macOS build (i.e. Opt, but not Shippable) reports figures such as

Reflow took 1230 ms; 22080 calls to getExtentOfChar took 4457 ms.

As a comparison, current Nightly (official Mozilla build) gives me

Reflow took 7597 ms; 22080 calls to getExtentOfChar took 7124 ms.

(FWIW, both Chrome and Safari still beat us by a considerable margin on the reflow, even after the patches here; OTOH the getExtentOfChar exercise brings them to their knees.)

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1765093 - Add perf tests for SVG text based on the testcase here. r=#layout-reviewers

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bbc194412080
patch 1 - Avoid measuring entire textrun in SVGTextFrame's GetBaselinePosition function. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/17d5218d84b7
patch 2 - Make ShrinkToLigatureBoundaries return whether any adjustment was made, so callers can skip redundant calls to partial-ligature code. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/6e5b336588d1
patch 3 - Also use the new gfxTextRun::GetLineHeightMetrics to optimize GetAscentAndDescentInAppUnits. r=emilio
https://hg.mozilla.org/integration/autoland/rev/4f24833ac254
Add perf tests for SVG text based on the testcase here. r=perftest-reviewers,emilio,sparky

Comment 13

[Cristian Tuns]

Backed out for causing web-platform-test failures on boundary-shaping-010.html

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | /css/css-text/boundary-shaping/boundary-shaping-010.html | Testing http://web-platform.test:8000/css/css-text/boundary-shaping/boundary-shaping-010.html == http://web-platform.test:8000/css/css-text/boundary-shaping/reference/boundary-shaping-010.ref.html

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a095433613cc
patch 1 - Avoid measuring entire textrun in SVGTextFrame's GetBaselinePosition function. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/271b6475c374
patch 2 - Make ShrinkToLigatureBoundaries return whether any adjustment was made, so callers can skip redundant calls to partial-ligature code. r=gfx-reviewers,lsalzman
https://hg.mozilla.org/integration/autoland/rev/aaffa1e26a69
patch 3 - Also use the new gfxTextRun::GetLineHeightMetrics to optimize GetAscentAndDescentInAppUnits. r=emilio
https://hg.mozilla.org/integration/autoland/rev/7b23b8f0b7a9
Add perf tests for SVG text based on the testcase here. r=perftest-reviewers,emilio,sparky

Comment 15

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/a095433613cc
https://hg.mozilla.org/mozilla-central/rev/271b6475c374
https://hg.mozilla.org/mozilla-central/rev/aaffa1e26a69
https://hg.mozilla.org/mozilla-central/rev/7b23b8f0b7a9

Comment 16

[Andrej (:aglavic)]

== Change summary for alert #34094 (as of Tue, 10 May 2022 17:01:31 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
14%	perf_reftest style-attr-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	3.90 -> 3.36
11%	perf_reftest_singletons style-attr-1.html	windows10-64-shippable-qr	e10s fission stylo webrender	3.14 -> 2.81
10%	perf_reftest style-attr-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	3.92 -> 3.52
10%	perf_reftest_singletons style-attr-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	3.99 -> 3.59

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34094

Comment 17

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

I managed to reproduce this bug on Firefox 101.0(20220526203855) on Windows 10 x64. Verified as fixed on Firefox 102.0b9(20220616185542) and Nightly 103.0a1(20220619215938) on Windows 10, macOS 11 and Ubuntu 22.04.