1670855 - 13.4 - 26.57% perf_reftest_singletons inline-style-cache-1.html / perf_reftest_singletons link-style-cache-1.html (macosx1014-64-shippable) regression on push 41d48f6233e24c6ce8244c9572203d06515bfe54 (Fri October 9 2020)

Status: RESOLVED DUPLICATE of bug 1671170

(Toolkit :: Storage, defect)

Description

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

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

Regressions:

27% perf_reftest_singletons link-style-cache-1.html macosx1014-64-shippable opt e10s stylo 1,306.03 -> 1,653.01
25% perf_reftest_singletons link-style-cache-1.html macosx1014-64-shippable opt e10s stylo 1,308.21 -> 1,633.97
13% perf_reftest_singletons inline-style-cache-1.html macosx1014-64-shippable opt e10s stylo 2,061.90 -> 2,338.13

Improvements:

19% perf_reftest_singletons link-style-cache-1.html windows7-32-shippable opt e10s stylo 739.48 -> 600.33
10% perf_reftest_singletons inline-style-cache-1.html windows7-32-shippable opt e10s stylo 1,778.55 -> 1,602.39

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

[Mark Hammond [:markh] [:mhammond]]

It doesn't really make any sense that this patch could cause this. The updated crates are only used by sync and by web-extensions, and they aren't exercised by the tests that regressed. Is there a chance this is a false positive? Backing out is an option, but realistically only a short-term one.

Comment 2

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

I did some retriggers to confirm the culprit, but the graph seems pretty clear.

Comment 3

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

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

Comment 4

[Pascal Chevrel:pascalc (PTO until April 26)]

Mark, shoud we prioritize this bug for 83? Thanks

Comment 5

[Mark Hammond [:markh] [:mhammond]]

(In reply to Pascal Chevrel:pascalc from comment #4)

Mark, shoud we prioritize this bug for 83? Thanks

I'm not sure what you are asking, but I don't think I'm going to be capable is discovering why updating a vendored rust crate that's only used for sync related functionality could cause this regression. This is also the only possible solution to the original crash, so I'm also not sure how to balance the tradeoff between that crash and this regression.

Comment 6

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

Unfortunately the retriggers reveal that this is the culprit: https://treeherder.mozilla.org/perf.html#/graphs?highlightAlerts=1&highlightedRevisions=4c2536efa6b875e1b2a93be8c539fb2557aab251&series=autoland,2131804,1,1&timerange=1209600&zoom=1602195188466,1602196024927,1275.3203575678342,1821.4475800173611

Comment 7

[Pascal Chevrel:pascalc (PTO until April 26)]

Unassigned S3 and 83 is in beta now, marking as fix-optional in case there is a simple fix we would want to uplift in early betas.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Looks like this regression is still there. Dave, can the perf team please help find someone to move this bug forward?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

This may be a dupe of bug 1671170. We should see if that fix also resolved these regressions once we have enough data points to compare with.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Perfherder confirms similar changes to the inverse of comment 0. Calling this a dupe of 1671170 (though it probably would be good to understand why Win7 saw such a big improvement from the original landing and regression from backing out).