| OLD | NEW |
| 1 # Addressing Performance Regressions | 1 # Addressing Performance Regressions |
| 2 | 2 |
| 3 The bisect bot just picked your CL as the culprit in a performance regression | 3 The bisect bot just picked your CL as the culprit in a performance regression |
| 4 and assigned a bug to you! What should you do? Read on... | 4 and assigned a bug to you! What should you do? Read on... |
| 5 | 5 |
| 6 [TOC] |
| 7 |
| 6 ## About our performance tests | 8 ## About our performance tests |
| 7 | 9 |
| 8 The [chromium.perf waterfall](perf_waterfall.md) is a continuous build which | 10 The [chromium.perf waterfall](perf_waterfall.md) is a continuous build which |
| 9 runs performance tests on dozens of devices across Windows, Mac, Linux, and | 11 runs performance tests on dozens of devices across Windows, Mac, Linux, and |
| 10 Android Chrome and WebView. Often, a performance regression only affects a | 12 Android Chrome and WebView. Often, a performance regression only affects a |
| 11 certain type of hardware or a certain operating system, which may be different | 13 certain type of hardware or a certain operating system, which may be different |
| 12 than what you tested locally before landing your CL. | 14 than what you tested locally before landing your CL. |
| 13 | 15 |
| 14 Each test has an owner, named in | 16 Each test has an owner, named in |
| 15 [this spreasheet](https://docs.google.com/spreadsheets/d/1xaAo0_SU3iDfGdqDJZX_jR
V0QtkufwHUKH3kQKF3YQs/edit#gid=0), | 17 [this spreadsheet](https://docs.google.com/spreadsheets/d/1xaAo0_SU3iDfGdqDJZX_j
RV0QtkufwHUKH3kQKF3YQs/edit#gid=0), |
| 16 who you can cc on a performance bug if you have questions. | 18 who you can cc on a performance bug if you have questions. |
| 17 | 19 |
| 18 ## Understanding the bisect results | 20 ## Understanding the bisect results |
| 19 | 21 |
| 20 The bisect bot spits out a comment on the bug that looks like this: | 22 The bisect bot spits out a comment on the bug that looks like this: |
| 21 | 23 |
| 22 ``` | 24 ``` |
| 23 === BISECT JOB RESULTS === | 25 === BISECT JOB RESULTS === |
| 24 Perf regression found with culprit | 26 Perf regression found with culprit |
| 25 | 27 |
| (...skipping 28 matching lines...) Expand all Loading... |
| 54 * **What regressed exactly?** The comment gives you several details: | 56 * **What regressed exactly?** The comment gives you several details: |
| 55 * **The benchmark that regressed**: Under `Bisect Details`, you can see | 57 * **The benchmark that regressed**: Under `Bisect Details`, you can see |
| 56 `Benchmark :`. In this case, the `system_health.common_desktop` | 58 `Benchmark :`. In this case, the `system_health.common_desktop` |
| 57 benchmark regressed. | 59 benchmark regressed. |
| 58 * **What platform did it regress on?** Under `Configuration`, you can find | 60 * **What platform did it regress on?** Under `Configuration`, you can find |
| 59 some details on the bot that regressed. In this example, it is a Mac Pro | 61 some details on the bot that regressed. In this example, it is a Mac Pro |
| 60 laptop. | 62 laptop. |
| 61 * **How do I run that locally?** Follow the instructions under | 63 * **How do I run that locally?** Follow the instructions under |
| 62 `To Run This Test`. But be aware that if it regressed on Android and | 64 `To Run This Test`. But be aware that if it regressed on Android and |
| 63 you're developing on Windows, you may not be able to reproduce locally. | 65 you're developing on Windows, you may not be able to reproduce locally. |
| 64 (See Debugging regressions below) | 66 (See [Debugging regressions](#Debugging-regressions) below) |
| 65 * **What is this testing?** Generally the metric | 67 * **What is this testing?** Generally the metric |
| 66 (`timeToFirstContentfulPaint_avg`) gives some information. If you're not | 68 (`timeToFirstContentfulPaint_avg`) gives some information. If you're not |
| 67 familiar, you can cc the [benchmark owner](https://docs.google.com/spreads
heets/d/1xaAo0_SU3iDfGdqDJZX_jRV0QtkufwHUKH3kQKF3YQs/edit#gid=0) | 69 familiar, you can cc the [benchmark owner](https://docs.google.com/spreads
heets/d/1xaAo0_SU3iDfGdqDJZX_jRV0QtkufwHUKH3kQKF3YQs/edit#gid=0) |
| 68 to ask for help. | 70 to ask for help. |
| 69 * **How severe is this regression?** There are different axes on which to | 71 * **How severe is this regression?** There are different axes on which to |
| 70 answer that question: | 72 answer that question: |
| 71 * **How much did performance regress?** The bisect bot answers this both | 73 * **How much did performance regress?** The bisect bot answers this both |
| 72 in relative terms (`Change : 15.25%`) and absolute terms | 74 in relative terms (`Change : 15.25%`) and absolute terms |
| 73 (`1010.02 -> 1164.04`). To understand the absolute terms, you'll need | 75 (`1010.02 -> 1164.04`). To understand the absolute terms, you'll need |
| 74 to look at the units on the performance graphs linked in comment #1 | 76 to look at the units on the performance graphs linked in comment #1 |
| (...skipping 96 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... |
| 171 entering the commit position of a change into this url: | 173 entering the commit position of a change into this url: |
| 172 `https://chromeperf.appspot.com/group_report?rev=YOUR_COMMIT_POS_HERE` | 174 `https://chromeperf.appspot.com/group_report?rev=YOUR_COMMIT_POS_HERE` |
| 173 All of these changes are generally changes found on a CL range, and may | 175 All of these changes are generally changes found on a CL range, and may |
| 174 not be attributable to your CL. You can bisect any of these to find if | 176 not be attributable to your CL. You can bisect any of these to find if |
| 175 your CL caused the improvement, just like you can bisect to find if it | 177 your CL caused the improvement, just like you can bisect to find if it |
| 176 caused the regression. | 178 caused the regression. |
| 177 * **Use finch trial results.** There are some types of changes that cannot | 179 * **Use finch trial results.** There are some types of changes that cannot |
| 178 be measured well in benchmarks. If you believe your case falls into this | 180 be measured well in benchmarks. If you believe your case falls into this |
| 179 category, you can show that end users are not affected via a finch trial. | 181 category, you can show that end users are not affected via a finch trial. |
| 180 See the "End-user metrics" section of | 182 See the "End-user metrics" section of |
| 181 [How does Chrome measure performance](how_does_chrome_measure_performance.
md) | 183 [How Chrome measures performance](how_does_chrome_measure_performance.md#E
nd_user-metrics) |
| 182 * **Your change is a critical correctness or security fix.** | 184 * **Your change is a critical correctness or security fix.** |
| 183 It's true that sometimes something was "fast" because it was implemented | 185 It's true that sometimes something was "fast" because it was implemented |
| 184 incorrectly. In this case, a justification should clarify the performance | 186 incorrectly. In this case, a justification should clarify the performance |
| 185 cost we are paying for the fix and why it is worth it. Some things to | 187 cost we are paying for the fix and why it is worth it. Some things to |
| 186 include: | 188 include: |
| 187 * **What did the benchmark regression cost?** Look at the | 189 * **What did the benchmark regression cost?** Look at the |
| 188 list of regressions in bug comment 1: | 190 list of regressions in bug comment 1: |
| 189 `https://chromeperf.appspot.com/group_report?bug_id=XXXX` | 191 `https://chromeperf.appspot.com/group_report?bug_id=XXXX` |
| 190 What is the absolute cost (5MiB RAM? 200ms on page load?) | 192 What is the absolute cost (5MiB RAM? 200ms on page load?) |
| 191 How many pages regressed? How many platforms? | 193 How many pages regressed? How many platforms? |
| (...skipping 13 matching lines...) Expand all Loading... |
| 205 possible that your change did not regress performance, and only appears to | 207 possible that your change did not regress performance, and only appears to |
| 206 be a problem because the test is measuring incorrectly. If this is the | 208 be a problem because the test is measuring incorrectly. If this is the |
| 207 case, you must explain clearly what the issue with the test is, and why you | 209 case, you must explain clearly what the issue with the test is, and why you |
| 208 believe your change is performance neutral. Please include data from traces | 210 believe your change is performance neutral. Please include data from traces |
| 209 or other performance tools to clarify your claim. | 211 or other performance tools to clarify your claim. |
| 210 | 212 |
| 211 **In all cases,** make sure to cc the [benchmark owner](https://docs.google.com/
spreadsheets/d/1xaAo0_SU3iDfGdqDJZX_jRV0QtkufwHUKH3kQKF3YQs/edit#gid=0) | 213 **In all cases,** make sure to cc the [benchmark owner](https://docs.google.com/
spreadsheets/d/1xaAo0_SU3iDfGdqDJZX_jRV0QtkufwHUKH3kQKF3YQs/edit#gid=0) |
| 212 when writing a justification and WontFix-ing a bug. If you cannot come to an | 214 when writing a justification and WontFix-ing a bug. If you cannot come to an |
| 213 agreement with the benchmark owner, you can escalate to benhenry@chromium.org, | 215 agreement with the benchmark owner, you can escalate to benhenry@chromium.org, |
| 214 the owner of speed releasing. | 216 the owner of speed releasing. |
| OLD | NEW |