1991943 - Try to optimize text-autospace processing

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, enhancement)

Description

[Jonathan Kew [:jfkthame]]

Enabling text-autospace: normal comes with significant performance cost, as it basically means we have to set TEXT_ENABLE_SPACING everywhere and lose out on the optimized measurement/rendering path that skips calling PropertyProvider::GetSpacing() and processing the resulting array of per-glyph spacing adjustments.

Until now, only some non-default properties (letter-spacing, word-spacing, text-align: justify, preserved tabs) required us to run the spacing process, but if text-autospace: normal is enabled as a default (or widely applied by sites) we end up having to do spacing work all the time.

Comment 1

[Jonathan Kew [:jfkthame]]

Try run with a couple of patches that might help a bit here:
https://treeherder.mozilla.org/jobs?repo=try&revision=adcfa1cc045c883a489a522a77ff7ae174267bd5

Base (unpatched) run for comparison:
https://treeherder.mozilla.org/jobs?repo=try&revision=f801b30911cd611d23c3974386401e4587505ab1

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1991943 - Use memset() to initialize spacing array to zero.

This is a trivial change that won't meaningfully move the perf needle,
but in theory the compiler might do something better with memset() than
the explicit loop over the buffer, explicitly assigning each element.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1991943 - Try to skip some text-autospace work if no CJK glyph runs are present.

For the GlyphRuns in a given gfxTextRun, we already track whether they are
associated with a CJK script, so we can use this knowledge to potentially
avoid some auto-spacing work when the textrun is known to be CJK-free. This
extends the previous idea of checking whether the character buffer is 2-byte,
and should let us skip the character-class processing in more cases.

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Here are the PerfCompare results for the two try runs in comment 3.

• MacOS
• Windows 11

speedometer3 Editor-TipTap tests have 10~13% improvement!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2a1d70136b8c
https://hg.mozilla.org/integration/autoland/rev/86a7737a505c
Use memset() to initialize spacing array to zero. r=layout-reviewers,emilio,TYLin
https://github.com/mozilla-firefox/firefox/commit/878d1288a772
https://hg.mozilla.org/integration/autoland/rev/aee907d92c5e
Try to skip some text-autospace work if no CJK glyph runs are present. r=layout-reviewers,dshin,TYLin

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/86a7737a505c
https://hg.mozilla.org/mozilla-central/rev/aee907d92c5e

Comment 7

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #5)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2a1d70136b8c
https://hg.mozilla.org/integration/autoland/rev/86a7737a505c
Use memset() to initialize spacing array to zero.
r=layout-reviewers,emilio,TYLin
https://github.com/mozilla-firefox/firefox/commit/878d1288a772
https://hg.mozilla.org/integration/autoland/rev/aee907d92c5e
Try to skip some text-autospace work if no CJK glyph runs are present.
r=layout-reviewers,dshin,TYLin

Perfherder has detected a browsertime performance change from push aee907d92c5ee9eacfdbf906e25cae62967f1321.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
20%	speedometer3 Editor-TipTap/Long/Sync	macosx1470-64-shippable	fission webrender	105.18 -> 84.34	Before/After
14%	speedometer3 Editor-TipTap/Long/Sync	windows11-64-24h2-shippable	fission webrender	47.41 -> 41.00
13%	speedometer3 Editor-TipTap/Long/total	windows11-64-24h2-shippable	fission webrender	49.15 -> 42.63
13%	speedometer3 Editor-TipTap/Long/Sync	macosx1500-aarch64-shippable	fission webrender	31.06 -> 27.17	Before/After
12%	speedometer3 Editor-TipTap/Long/total	macosx1500-aarch64-shippable	fission webrender	31.73 -> 27.81	Before/After
...	...	...	...	...	...
6%	speedometer3 Editor-TipTap/Long/total	android-hw-a55-14-0-aarch64-shippable	webrender	89.71 -> 84.72

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 47007

The following documentation link provides more information about this command.

Comment 8

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #5)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2a1d70136b8c
https://hg.mozilla.org/integration/autoland/rev/86a7737a505c
Use memset() to initialize spacing array to zero.
r=layout-reviewers,emilio,TYLin
https://github.com/mozilla-firefox/firefox/commit/878d1288a772
https://hg.mozilla.org/integration/autoland/rev/aee907d92c5e
Try to skip some text-autospace work if no CJK glyph runs are present.
r=layout-reviewers,dshin,TYLin

Perfherder has detected a talos performance change from push aee907d92c5ee9eacfdbf906e25cae62967f1321.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
9%	perf_reftest_singletons resize-lengthy-textarea.html	macosx1470-64-shippable	e10s fission stylo webrender	162.70 -> 148.76

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 47008

The following documentation link provides more information about this command.