1742874 - Adjust the content of a perf regression message; it is too harsh and can turn off well meaning engineers

Status: NEW

(Testing :: Performance, enhancement, P3)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

for all perf regressions, there is an auto generated template to get a bug on file. Text goes like this:

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

...

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.

If there is a way to write this friendlier or less finger pointing, that would be helpful. I recommend getting feedback from seasoned engineers, junior engineers, volunteers, men, women, and a variety of backgrounds. A little bit could go a long way.

Comment 1

[Greg Mierzwinski [:sparky]]

Can you point to specific parts that you think are too aggressive? I think the main line we'd want to change is this one as the other sentences seem neutral to me:

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.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

I think that specific line could use some attention. It is very bad cop style (and I think I wrote it!)

Comment 3

[Mark Banner (:standard8)]

Having hit this a few times, and spoken to someone else about their experience recently, I think there are a few issues with the performance regression handling process, which starts with the bug filing:

• The text references "we need your help", but doesn't say how to get help.
  • I'm experienced in the code base, but it still feels a struggle to find the right people to get help. For a new contributor this will be much harder.
  • It feels like the onus is on the patch filer to sort everything out and fix it on their own, even when they have no idea how their patch might have affected the regressing test.
  • Two suggestions:
    • Have a guide on how to go about investigating performance issues & everything surrounding (looking at graphs, who to talk to etc).
    • Have a pointer to a matrix channel where people can ask for help, and ensure the channel is actively monitored.
• The graphs are not directly linked - you have to finding the link to the alert information, then find the graph icon. I've had a few cases where regressions have been misfiled, and double-checking seems to be the first thing you want to be able to do.
  • Suggestion: Direct link to the graphs from the tables.
• Many of the linked talos tests seem under detailed and you need to find the owners to find out more. For example google-docs-canvas specifies that it measures fnbpaint, fcp, dcf, loadtime but I have not idea what the first three of those are.
  • Suggestion: Review source docs & check that terminology is described.
• There's no indication how a call is potentially made for if a regression is acceptable or not.
  • This could be incorporated in the guide mentioned above, or a separate document.

Comment 4

[Michelle Goossens [:masterwayz]]

I agree with Standard8 and sparky. It's like, "fix this or else we'll back it out, but you're on your own" which is really hard as a contributor since I don't know anything about perf things, and there's nothing else helpful in the message, other than a stress-inducing "fix this now" message, which whenever I get those it's quite bad on my mental health.