1813515 - 28.02% twitch-animation (OSX) regression on Wed January 25 2023

Status: RESOLVED FIXED

(Core :: Performance, defect)

Description

[Alex Finder]

Perfherder has detected a browsertime performance regression from push eadea8a10f38cf8643042bdcaa743dcc1cbd26ab. 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)	Performance Profiles
28%	twitch-animation	macosx1015-64-shippable-qr	fission webrender	17.16 -> 21.97	Before/After

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) may be backed out in accordance with our regression policy.

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

For more information on performance sheriffing please see our FAQ.

Comment 1

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

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

Comment 2

[Andrew Creskey [:acreskey]]

(In reply to Alex Finder from comment #0)

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

Hi Alex,
The change from Bug 1810871 does not look to be included in the referenced push:
eadea8a10f38cf8643042bdcaa743dcc1cbd26ab.
I only see Bug 1639052.

Comment 3

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

The bug is marked as tracked for firefox111 (nightly). We have limited time to fix this, the soft freeze is in 9 days. However, the bug still isn't assigned.

:fdoty, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 4

[Donal Meehan [:dmeehan]]

:afinder re: Comment 2, looks like Bug 1639052 should be the regressor?

Comment 5

[Andrew Creskey [:acreskey]]

Looking at the change from Bug 1639052, it also doesn't seem likely that it caused the regression.
is it possible to re-run the tests around this regression?

Comment 6

[Frank Doty [:fdoty]]

clearing NI until the above tests are rerun to determine a path forward

Comment 7

[Alex Finder]

(In reply to Frank Doty [:fdoty] from comment #6)

clearing NI until the above tests are rerun to determine a path forward

Hi Frank, Andrew and Donal! Sorry for the late reply. Looking at the graph around the last datapoint from the previous trend and the first datapoint in the new trend change, it looks like the regression was introduced around here in the graph. There's also a perfcompare link that suggests a regression was introduced betwen c26685da3ad7 and 9d53e3496082 for twitch on MacOS. Please let me know if I'm missing something or there is other information I can help provide.

Comment 8

[Andrew Creskey [:acreskey]]

Thanks Alex.
Bug 1810871 only adds some low-overhead performance metrics.
But I see the perfcompare link, so that is interesting.
Let me see if removing Bug 1810871 affects twitch-animation (OSX).

Comment 9

[Andrew Creskey [:acreskey]]

Oh, I think I see what's happening here.

This test, twitch-animation is scored based on the sum of the PerfStats (lower is better).
And Bug 1810871 adds three new PerfStats, so of course the score gets larger, signalling a regression.

Here's a backout highlighting the counted metrics.

Sparky -- is this correct, we want twitch-animation to be scored on all the Perf Stats?
From this code it looks like twitch-animation should only be scored from the 'run' value?

Comment 10

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

This is a reminder regarding comment #3!

The bug is marked as tracked for firefox111 (nightly). We have limited time to fix this, the soft freeze is in 3 days. However, the bug still isn't assigned.

Comment 11

[Andrew Creskey [:acreskey]]

From looking at the subtest results (see Comment 9), this isn't an actual regression -- we've simply added more metrics that are counted.
It's also not clear to me why the given test, twitch-animation is based on all perfStats, some have nothing to do with animation.

Comment 12

[Frank Doty [:fdoty]]

From Comment 11, what additional information is needed to get this closed out prior to the soft freeze?

Comment 13

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

This is a reminder regarding comment #3!

The bug is marked as tracked for firefox111 (nightly). We have limited time to fix this, the soft freeze is in 2 days. However, the bug still isn't assigned.

Comment 14

[Greg Mierzwinski [:sparky]]

Thanks for looking into it :acreskey! I think it's because of these lines of code: https://searchfox.org/mozilla-central/source/testing/raptor/raptor/output.py#471-476

Essentially, we are taking a geomean of all the values available to us there. Instead, we should only be selecting the run metric as the summary value. I'll make a patch to fix this.

Comment 15

[Greg Mierzwinski [:sparky]]

Just a warning here that this upcoming change is going to cause a very large expected regression in the summary metric, here's an example to show the difference between the existing summary value, and the new one: https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&series=autoland,4408688,1,13&series=autoland,4408706,1,13&timerange=31536000&zoom=1673142885262,1675185493152,11607.81017068147,12954.11831131969

Comment 16

[Greg Mierzwinski [:sparky]]

Attachment: Bug 1813515 - Use the run metric as the twitch-animation summary value. r?#perftest

This patch fixes an issue with twitch-animation where the summary value was being calculated as the geomean of all the subtests when it should only display the geomean of the run metric.

Comment 17

[Andrew Creskey [:acreskey]]

Appreciate the quick fix, Greg!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/970bcb73de2c
Use the run metric as the twitch-animation summary value. r=perftest-reviewers,afinder

Comment 19

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/970bcb73de2c