TestEngineering/Performance/RegressionBugsHandling - Revision history

Gsquelart: Updated link to build metrics.

2021-08-26T13:50:59Z

Updated link to build metrics.

← Older revision		Revision as of 13:50, 26 August 2021
Line 20:		Line 20:
	== Build metrics regressions ==		== Build metrics regressions ==

	More information about build metrics results can be found [https://~~developer~~.mozilla.org/~~en-US~~/~~docs~~/~~Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Build_Metrics~~ here]		More information about build metrics results can be found [https://firefox-source-docs.mozilla.org/performance/build_metrics/build_metrics.html here]

	= Policies =		= Policies =

Davehunt at 11:53, 16 March 2021

2021-03-16T11:53:06Z

← Older revision		Revision as of 11:53, 16 March 2021
Line 10:		Line 10:
	== Talos regressions ==		== Talos regressions ==

	For more information about Talos tests please ~~go to~~ [~~[TestEngineering~~/~~Performance~~/~~Talos~~/~~Tests\|Talos tests]~~] and for more information about running Talos tests please go to [[TestEngineering/Performance/Talos/Running\|Running Talos]].		For more information about Talos tests please visit the [https://firefox-source-docs.mozilla.org/testing/perfdocs/talos.html#test-definitions test documentation], and for more information about running Talos tests please go to [[TestEngineering/Performance/Talos/Running\|Running Talos]].

	Guide to handling Talos regressions you can find [[TestEngineering/Performance/Running_performance_tests\|here]].		Guide to handling Talos regressions you can find [[TestEngineering/Performance/Running_performance_tests\|here]].

Alexandrui: /* Handling of Performance Regression Bugs */

2020-05-19T06:49:36Z

Handling of Performance Regression Bugs

← Older revision		Revision as of 06:49, 19 May 2020
Line 1:		Line 1:
	= Handling of Performance Regression Bugs =		= Handling of Performance Regression Bugs =
	~~Bwloe~~ you'll find references to information about how you can investigate a potential regression for each framework.		Below you'll find references to information about how you can investigate a potential regression for each framework.

	== Raptor regressions ==		== Raptor regressions ==

Alexandrui: /* Handling of Performance Regression Bugs */

2020-05-19T06:48:49Z

Handling of Performance Regression Bugs

← Older revision		Revision as of 06:48, 19 May 2020
Line 1:		Line 1:
	= Handling of Performance Regression Bugs =		= Handling of Performance Regression Bugs =
			Bwloe you'll find references to information about how you can investigate a potential regression for each framework.

	== Raptor regressions ==		== Raptor regressions ==

Alexandrui: /* Handling of Performance Regression Bugs */

2020-05-19T06:47:43Z

Handling of Performance Regression Bugs

@@ Line 1: / Line 1: @@
 = Handling of Performance Regression Bugs =
-We use different policies for handling large and small regressions.
+== Raptor regressions ==
 == Policy #1: Backouts within 48-hours for large regressions ==

Alexandrui: /* Running performance tests */

2020-05-13T05:59:39Z

Running performance tests

← Older revision		Revision as of 05:59, 13 May 2020
Line 54:		Line 54:

	==== See also ====		==== See also ====
	* [https://wiki.mozilla.org/~~Buildbot~~/~~Talos~~/Sheriffing ~~Regression sheriffing~~] - how are regressions detected and associated with patches.		* [https://wiki.mozilla.org/TestEngineering/Performance/Sheriffing/Workflow Sheriffing workflow] - how are regressions detected and associated with patches.

Alexandrui at 12:55, 12 May 2020

2020-05-12T12:55:17Z

← Older revision		Revision as of 12:55, 12 May 2020
Line 3:		Line 3:
	We use different policies for handling large and small regressions.		We use different policies for handling large and small regressions.

	= Policy #1: Backouts within 48-hours for large regressions =		== Policy #1: Backouts within 48-hours for large regressions ==

	The perf team and the A-Team are testing out a new policy: backing out patches that cause significant performance regressions within 48 hours.		The perf team and the A-Team are testing out a new policy: backing out patches that cause significant performance regressions within 48 hours.
Line 20:		Line 20:
	The A-Team has been working hard on improving the tools for understanding performance regressions (e.g. Perfherder), and we think debugging a performance regression is a much less painful process these days. For example, there is now a highly usable view to visualize the comparison between a proposed patch against a baseline revision at https://treeherder.mozilla.org/perf.html#/comparechooser.		The A-Team has been working hard on improving the tools for understanding performance regressions (e.g. Perfherder), and we think debugging a performance regression is a much less painful process these days. For example, there is now a highly usable view to visualize the comparison between a proposed patch against a baseline revision at https://treeherder.mozilla.org/perf.html#/comparechooser.

	= Policy #2: Policy for smaller regressions =		== Policy #2: Policy for smaller regressions ==

	===== When a performance regression is detected and the patch which caused it is identified =====		===== When a performance regression is detected and the patch which caused it is identified =====
Line 49:		Line 49:
	The bottom line is that we want a visible decision on each case where the cause for the regression is known.		The bottom line is that we want a visible decision on each case where the cause for the regression is known.

	====See also====		= Running performance tests =

			If, for some reason, the graph that generated the regression alert is not conclusive enough, there's a [[TestEngineering/Performance/RunningTests\| Guide to running performance tests]] that can help you so additional pushes to try to find out what's missing.

			==== See also ====
	* [https://wiki.mozilla.org/Buildbot/Talos/Sheriffing Regression sheriffing] - how are regressions detected and associated with patches.		* [https://wiki.mozilla.org/Buildbot/Talos/Sheriffing Regression sheriffing] - how are regressions detected and associated with patches.

Alexandrui: Created page with "= Handling of Performance Regression Bugs = We use different policies for handling large and small regressions. = Policy #1: Backouts within 48-hours for large regressions =..."

2020-05-12T12:47:29Z

Created page with "= Handling of Performance Regression Bugs = We use different policies for handling large and small regressions. = Policy #1: Backouts within 48-hours for large regressions =..."

New page

= Handling of Performance Regression Bugs =

We use different policies for handling large and small regressions.

= Policy #1: Backouts within 48-hours for large regressions =

The perf team and the A-Team are testing out a new policy: backing out patches that cause significant performance regressions within 48 hours.

Only regressions of 10% or more will face backout.

===== Why =====
We want to test a 48-hour backout policy because we noticed that patch authors tend not to address performance regression bugs for days. If a regression sits in the tree for days, it becomes difficult to back it out, and it becomes much more likely the regression will end up riding the trains to release by default.

This new policy is more aggressive. We think a patch that regresses performance significantly should be backed out quickly, and re-landed when its performance is acceptable.

===== How do the 48-hour backouts work? =====

The A-Team perf sheriffs will create a performance regression bug as soon as a regression is confirmed via [[TestEngineering/Performance/Sheriffing/Workflow|the sheriffing workflow]]. The patch author and reviewer will be CC’ed, and if they don’t provide an explanation for why the regression is acceptable, the patch will be backed out. The goal is to back out unjustified regressions within 48 hours of them landing. We’d like to give the patch author about 24 hours to reply after the regression bug is filed.

The A-Team has been working hard on improving the tools for understanding performance regressions (e.g. Perfherder), and we think debugging a performance regression is a much less painful process these days. For example, there is now a highly usable view to visualize the comparison between a proposed patch against a baseline revision at https://treeherder.mozilla.org/perf.html#/comparechooser.

= Policy #2: Policy for smaller regressions =

===== When a performance regression is detected and the patch which caused it is identified =====
* Whoever landed the patch is responsible to drive a decision on whether or not the regression is acceptable.
* Such decision should preferably involve a module owner/peer of the regressed module.
* The owner of the offending patch must respond at the bug within [https://groups.google.com/forum/#!searchin/mozilla.dev.platform/joel$20regression/mozilla.dev.platform/HkfaOTtAFVo/6L1E9GiOGGIJ 3 business days], and a decision has to be made within two weeks, otherwise the offending patch may be backed out.

===== If the decision was to try and fix the regression, preferably a time frame and an owner would be provided as well. =====

===== It's OK to decide to accept a performance regression for valid reasons, including =====
* There is other higher priority work, the patch's functionality is absolutely required, and no one else is available to investigate in the foreseeable future.
* The magnitude of the regression is too small to warrant the effort required for an investigation.
* The performance improvements from the patch outweigh the regression on the regressing test
* The functionality provided by the patch is absolutely required (e.g. a security patch) or otherwise out-weighs the regression.
* etc

Obviously, if someone still wants to fix the regression or otherwise improve the performance even if a valid reason to not do so exists, that would be welcome.

===== Invalid reasons for closing a performance regression bug =====
* "I don't understand how my patch could regress this test"
** If you think your patch is not to blame, we can re-trigger the jobs to confirm the before-and-after behavior. Talk to the person who filed the bug, they would be happy to help you figure out if the regression is real and if it is indeed your patch that is responsible.
** It's up to the patch author to contact the right people (test authors, another team, etc) to figure out why the test regressed.
** As a general rule, we're all responsible for the performance of the code we write, even if the regression is in an unrelated part of the codebase.
* "I don't have time to deal with this right now"
** This one can be valid (see "valid reasons" above). However, if the regression is quite large and you're pressed for time, consider backing the patch out and re-landing it when you have time to study its performance impact.
** If the patch can't be backed out and you don't have time to work on it at the moment, make sure to file a follow-up bug to fix the regression, assign yourself and provide a timeline for the investigation & fixing. We should fix regressions before they reach Beta/Release users.

The bottom line is that we want a visible decision on each case where the cause for the regression is known.

====See also====
* [https://wiki.mozilla.org/Buildbot/Talos/Sheriffing Regression sheriffing] - how are regressions detected and associated with patches.