1753436 - 13.06 - 3.14% glterrain / tscrollx + 8 more (Linux, OSX, Windows) regression on Mon January 31 2022

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Andrej (:aglavic)]

Perfherder has detected a talos performance regression from push c55e3a40a4de20482f056a3de9e9823fb8c82768. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
13%	glterrain	windows10-64-qr	e10s stylo webgl-ipc webrender	1.84 -> 2.08
12%	glterrain	macosx1015-64-shippable-qr	e10s fission stylo webgl-ipc webrender	1.82 -> 2.04
11%	glterrain	macosx1015-64-shippable-qr	e10s stylo webrender	1.82 -> 2.03
11%	glterrain	macosx1015-64-shippable-qr	e10s fission stylo webrender	1.84 -> 2.04
11%	glterrain	windows10-64-shippable-qr	e10s fission stylo webrender	1.71 -> 1.90
9%	glterrain	linux1804-64-shippable-qr	e10s stylo webrender	5.69 -> 6.21
9%	tscrollx	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	1.10 -> 1.19
8%	glterrain	macosx1015-64-shippable-qr	e10s stylo webgl-ipc webrender	1.84 -> 1.99
7%	glterrain	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	3.46 -> 3.70
6%	tp5o_scroll	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	1.90 -> 2.01
3%	tscrollx	linux1804-64-shippable-qr	e10s fission stylo webrender	1.41 -> 1.45

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
10%	perf_reftest some-descendants-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	4.68 -> 4.20
5%	tart	macosx1015-64-shippable-qr	e10s fission stylo webrender	2.11 -> 1.99
5%	tart	windows10-64-shippable-qr	e10s fission stylo webrender-sw	2.49 -> 2.35
5%	tart	windows10-64-shippable-qr	e10s fission stylo webrender	2.59 -> 2.47
4%	tart	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	2.02 -> 1.93
2%	tresize	linux1804-64-shippable-qr	e10s fission stylo webrender	17.53 -> 17.10

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the offending patch(es) will be backed out in accordance with our regression policy.

For more information on performance sheriffing please see our FAQ.

Comment 1

[Kelsey Gilbert [:jgilbert]]

I'm not worried about the glterrain test regressions here, but thanks for the heads-up!

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Thank you :jgilbert.

As for scroll related tests, I am very confused. Indeed bug 1571758 has some negative performance impacts on scroll operations. With some additional pref runs, it turns out that bug 1571758 regressed sw-wr cases significantly. I have no idea why bug 1571758 had much impacts on sw-wr. Keeping NI to me.

Comment 3

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #33165 (as of Tue, 01 Feb 2022 21:54:14 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
4%	pinterest loadtime	linux1804-64-shippable-qr	fission warm webrender	1,504.88 -> 1,567.17
3%	cnn loadtime	linux1804-64-shippable-qr	fission warm webrender	950.88 -> 975.58
2%	expedia loadtime	linux1804-64-shippable-qr	fission warm webrender	1,187.21 -> 1,216.29

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
18%	instagram LastVisualChange	android-hw-p2-8-0-android-aarch64-shippable-qr	cold webrender	982.29 -> 803.58

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

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1571758

Comment 5

[Hiroyuki Ikezoe (:hiro)]

After a bunch of performance runs I manged to identify what caused this regression. The part is this check in set_scroll_offsets. Before bug 1571758 we just compared scroll offsets and then if the newly scroll offset is different from the previous one, we rebuild hit testing tree. And now we compare both of scroll offsets and generations, thus even if the new scroll offset hasn't been changed and if the generation is different, then we rebuild hit testing tree.

A solution I can think of is this bumping the generation with some conditions, just like this.

With the change, scroll related perf tests look mostly same as before.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1753436 - Bump up APZ scroll generation only if it's necessary. r?botond

APZ scroll generation doesn't need to be bumped up if the target APZC's
scroll position isn't changed.

Comment 7

[Botond Ballo [:botond]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #5)

Before bug 1571758 we just compared scroll offsets and then if the newly scroll offset is different from the previous one, we rebuild hit testing tree. And now we compare both of scroll offsets and generations, thus even if the new scroll offset hasn't been changed and if the generation is different, then we rebuild hit testing tree.

Nice example of a perf test catching a real perf regression introduced by a patch!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a623f57acf3b
Bump up APZ scroll generation only if it's necessary. r=botond

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/a623f57acf3b

Comment 10

[Pascal Chevrel:pascalc]

Hiro, this is a 98 regression, should we uplift the fix to beta? Thanks

Comment 11

[Hiroyuki Ikezoe (:hiro)]

I'd expect this regression won't be noticeable in the wild because these affected tests are run with ASAP mode, it's unusual setup.

Comment 12

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Pulsebot from comment #8)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a623f57acf3b
Bump up APZ scroll generation only if it's necessary. r=botond

== Change summary for alert #33266 (as of Mon, 14 Feb 2022 06:43:54 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
7%	sina loadtime	android-hw-g5-7-0-arm7-shippable-qr	cold webrender	3,411.73 -> 3,660.17

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
4%	expedia loadtime	linux1804-64-shippable-qr	fission warm webrender	1,231.98 -> 1,186.71
3%	cnn-nav.world loadtime	linux1804-64-shippable-qr	cold fission webrender	1,021.46 -> 988.33
3%	facebook fcp	linux1804-64-shippable-qr	cold fission webrender	1,103.10 -> 1,069.50
3%	facebook LastVisualChange	linux1804-64-shippable-qr	cold fission webrender	2,021.67 -> 1,963.33
3%	google-docs-canvas dcf	linux1804-64-shippable-qr	cold fission webrender	983.21 -> 958.50
...	...	...	...	...
2%	google-docs-canvas dcf	linux1804-64-shippable-qr	fission warm webrender	682.23 -> 667.54

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

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Alexandru, do I have to do some actions for the sina case regression?

Comment 14

[Hiroyuki Ikezoe (:hiro)]

this change which landed just after my push looks highly suspicious. (I will trigger the perf test for the commit)

Comment 15

[Hiroyuki Ikezoe (:hiro)]

(In reply to Alexandru Ionescu (needinfo me) [:alexandrui] from comment #12)

(In reply to Pulsebot from comment #8)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a623f57acf3b
Bump up APZ scroll generation only if it's necessary. r=botond

This revision doesn't look the culprit;
https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&selected=3757607,1488771909&series=autoland,3757607,1,13&timerange=1209600

Comment 16

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #14)

this change which landed just after my push looks highly suspicious. (I will trigger the perf test for the commit)

yep, it is; https://treeherder.mozilla.org/perfherder/compare?originalProject=autoland&newProject=autoland&newRevision=4a3025a77f16de44fe2c9f1170f8b3925a03a825&framework=13&originalRevision=a623f57acf3bcc5219e8ce383008f1d8ec609510&page=1&showOnlyImportant=1

Comment 17

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #15)

This revision doesn't look the culprit;
https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&selected=3757607,1488771909&series=autoland,3757607,1,13&timerange=1209600

Yup, I reassigned it, thanks!

Edit: I retriggered. hiro, can you please look again at the greph? Feel free to retrigger more if anything unclear to you. just use max 5 retriggers per revision.