Closed Bug 1813515 Opened 1 year ago Closed 1 year ago

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

Categories

(Core :: Performance, defect)

Desktop
macOS
defect

Tracking

()

RESOLVED FIXED
111 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox109 --- unaffected
firefox110 --- unaffected
firefox111 + fixed

People

(Reporter: afinder, Assigned: sparky)

References

(Regression)

Details

(Keywords: perf, perf-alert, regression)

Attachments

(1 file)

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.

Flags: needinfo?(acreskey)

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

(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.

Flags: needinfo?(acreskey)

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.

Flags: needinfo?(fdoty)

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

Flags: needinfo?(afinder)

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?

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

Flags: needinfo?(fdoty)

(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.

Flags: needinfo?(afinder)

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).

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?

Flags: needinfo?(gmierz2)

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.

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.

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

Flags: needinfo?(afinder)

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.

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.

Flags: needinfo?(gmierz2)

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

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.

Assignee: nobody → gmierz2
Status: NEW → ASSIGNED

Appreciate the quick fix, Greg!

Pushed by gmierz2@outlook.com:
https://hg.mozilla.org/integration/autoland/rev/970bcb73de2c
Use the run metric as the twitch-animation summary value. r=perftest-reviewers,afinder
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch
Flags: needinfo?(afinder)
Regressions: 1817423
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: