1648687 - 16.44 - 2809.13% perf_reftest (linux64-shippable|-qr, macosx1014-64-shippable, windows10-64-shippable|-qr, windows7-32-shippable) regression on push ce5cdfc5d7cfb19af1044a5e1b7e93eedeb5e8be (Tue June 23 2020)

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

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

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

Regressions:

2809% perf_reftest recompute-position-vertical-rl.html macosx1014-64-shippable opt e10s stylo 0.42 -> 12.22
2167% perf_reftest recompute-position-vertical-rl.html linux64-shippable opt e10s stylo 0.48 -> 10.81
1726% perf_reftest recompute-position-vertical-rl.html windows10-64-shippable opt e10s stylo 0.57 -> 10.47
1661% perf_reftest recompute-position-vertical-rl.html windows7-32-shippable opt e10s stylo 0.68 -> 11.97
1446% perf_reftest recompute-position-vertical-rl.html linux64-shippable-qr opt e10s stylo 0.77 -> 11.83
1427% perf_reftest recompute-position-vertical-rl.html windows10-64-shippable-qr opt e10s stylo 0.73 -> 11.09
20% perf_reftest windows10-64-shippable-qr opt e10s stylo 1.48 -> 1.77
20% perf_reftest windows7-32-shippable opt e10s stylo 1.37 -> 1.65
16% perf_reftest windows10-64-shippable-qr opt e10s stylo 1.52 -> 1.77

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

[Kartikaya Gupta (email:[e-mail])]

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

Perfherder has detected a talos performance regression from push ce5cdfc5d7cfb19af1044a5e1b7e93eedeb5e8be.

This push is for a different bug with a different author. Can you clarify which one is the correct push?

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Looks like cc56a4cc04156b0c58944eb32ddf4f9592d2c0cd is the culprit, which is not unreasonable. The work I'm doing in bug 1647034 will probably help some. If it doesn't fix it entirely I can investigate more at that point.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Also as a fallback plan for uplift to 79 (since we're about to switch to 80 on nightly) we should be able to turn off apz.mvm.force-enabled and readjust the necessary crashtest assertions. I did a try push that shows it's green for correctness, and I've scheduled the perf-reftest talos jobs on it to verify it will fix the regressions.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

With my current WIP for bug 1647034: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=68c073816771e28e68cbc627278a9ab04d222dbb&framework=1&selectedTimeRange=172800

With the MVM disabled: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=29d21576c78a76da6898c4a339700df893fe2340&framework=1&selectedTimeRange=172800

Both seem pretty good and fix this regression.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

With bug 1647034 on autoland I see the graphs going back to their previous values. So I'll close this bug once that merges to m-c. However for uplifting to 79 I'll make a patch to disable the MVM pref, because I think that's a safer fix than uplifting the new patches that landed on 80.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1648687 - Disable the visual-viewport-only MVM for beta 79. r?tnikkel

This is the low-risk fix for landing on beta 79 to restore original
performance.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 9160394 [details]
Bug 1648687 - Disable the visual-viewport-only MVM for beta 79. r?tnikkel

Beta/Release Uplift Approval Request

• User impact if declined: Slight performance regression
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Disables the code that introduced the performance regression. Note that the regression has been fixed in 80 but it's safer to disable the offending code than to uplift the fixes for fixing the perf regression in 79.
• String changes made/needed:

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9160394 [details]
Bug 1648687 - Disable the visual-viewport-only MVM for beta 79. r?tnikkel

Approved for 79.0b2, thanks.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1eea2bfc562f