1989851 - More performance optimization for text-autospace

Status: RESOLVED FIXED

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

Description

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

As noted in bug 1987997 comment 8, svg-text-textLength-1.html can cause perf_reftest_singletons talos test suites time out intermittently on Linux, which is blocking enabling text-autospace on Nightly.

The performance regression occurs after bug 1986837. Specifically, the 8-bit text optimization is removed in Bug 1986837 patch 2, and determining the autospace in the preceding content takes more time.

Comment 1

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

Attachment: Bug 1989851 Part 1 - Avoid unnecessary findPrecedingClass() calls. r?jfkthame,#layout-jp-market-reviewers

valueOr() unconditionally evaluates its parameter, so we should use
valueOrFrom() to call findPrecedingClass() only when needed.
See the documentation:
https://searchfox.org/firefox-main/rev/36ebbdf6fbbfd0b40a607212ec843992b5d43e92/mfbt/Maybe.h#504-509

With layout.css.text-autospace.enabled=true, on my local Linux machine, the
running time of
testing/talos/talos/tests/perf-reftest-singletons/svg-text-textLength-1.html
drops from 29,425ms to 22,279ms (~24% faster).

Comment 2

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

Attachment: Bug 1989851 Part 2 - Check mWordSpacing before calling IsCSSWordSpacingSpace(). r?jfkthame,#layout-jp-market-reviewers

If word-spacing is zero, we don't need to call IsCSSWordSpacingSpace() at
all.

With layout.css.text-autospace.enabled=true, on my local Linux machine, the
the running time of
testing/talos/talos/tests/perf-reftest-singletons/svg-text-textLength-1.html
drops from 22,279 ms to 19,274 ms (~13% faster).

Comment 3

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

Attachment: Bug 1989851 Part 3 - Run text-autospace for 1-byte text only at the start of the frame. r?jfkthame,#layout-jp-market-reviewers

The second and subsequent characters cannot be ideographs in a 1-byte text.

With layout.css.text-autospace.enabled=true, on my local Linux machine, the
running time of
testing/talos/talos/tests/perf-reftest-singletons/svg-text-textLength-1.html
drops from 19,274 ms to 13,052 ms (~32% faster).

Comment 4

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

Here's a performance comparison for perf_reftest_singletons on Windows (base = text-autospace enabled, new = text-autospace enabled + the patches in this bug).

The two subtests most impacted by text-autospace show substantial gains:

• bidi-resolution-1.html: ~15% improvement
• svg-text-textLength-1.html: ~42% improvement

Comment 5

[Jonathan Kew [:jfkthame]]

The optimizations here are great, thank you! Hopefully this will let us turn on the pref for wider testing without further problems.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/eafe10d94e80
https://hg.mozilla.org/integration/autoland/rev/fc4ef6354775
Part 1 - Avoid unnecessary findPrecedingClass() calls. r=jfkthame,layout-jp-market-reviewers,layout-reviewers,emilio
https://github.com/mozilla-firefox/firefox/commit/ac8e030f7c7c
https://hg.mozilla.org/integration/autoland/rev/97f05001d655
Part 2 - Check mWordSpacing before calling IsCSSWordSpacingSpace(). r=jfkthame,layout-jp-market-reviewers,layout-reviewers
https://github.com/mozilla-firefox/firefox/commit/7380fbe57e1f
https://hg.mozilla.org/integration/autoland/rev/6c63924355bd
Part 3 - Run text-autospace for 1-byte text only at the start of the frame. r=jfkthame,layout-jp-market-reviewers,layout-reviewers

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/fc4ef6354775
https://hg.mozilla.org/mozilla-central/rev/97f05001d655
https://hg.mozilla.org/mozilla-central/rev/6c63924355bd