dotnet / performance

This repo contains benchmarks used for testing the performance of all .NET Runtimes
MIT License
691 stars 269 forks source link

Align auto-filed issue labels with the dotnet/runtime repo labels #2808

Open jeffhandley opened 1 year ago

jeffhandley commented 1 year ago

To improve the quality of issues transferred into the dotnet/runtime repository, we should align the labels between the perf-autofiling-issues repository and the runtime repository.

Here's the list of label changes to make that would bring the repositories into alignment:

  1. alpine 3.12 and alpine 3.15 --> os-alpine
  2. arm64 --> arch-arm64
  3. WASM, CompilationMode=wasm, and the current label, which has a typo, CompliationMode=wasm --> arch-wasm
  4. CoreClr --> runtime-coreclr
  5. Mono --> runtime-mono
  6. ubuntu 18.04 and ubuntu 20.04 --> os-linux
  7. Windows 10.* --> os-windows
  8. x64 --> arch-x64
  9. x86 --> arch-x86
  10. Look Again --> needs-further-triage

There's a typo in Needs Trasnfer that could be fixed. The following labels could be deleted:

  1. documentation
  2. duplicate
  3. enhancement
  4. good first issue
  5. help wanted
  6. invalid
  7. question
  8. test
  9. test2
  10. wontfix
lewing commented 1 year ago

pertains to https://github.com/dotnet/performance/issues/2750

caaavik-msft commented 1 year ago

We currently use the labels to tell whether or not the autofile issues were successfully filed, however I am changing it so that instead we put the run information inside a hidden markdown comment and read that programmatically instead of the labels. This means that we can start setting the labels to whatever we want.

I see as per #2809, we want to remove the Regression/Improvement labels which should be fine since the title of the issue states whether it tracks regressions or improvements. For the list of label changes you mentioned I have changed them locally aside from Look Again as that one is set manually and needs to be configured inside GitHub.

I went through the list of all the other labels we have currently in this repo and here are the ones that we need to figure out what to do with them:

Without these labels it's not easy to see any of the above, so we either need to keep them as labels (and rename them to something more standard), put the data in the issue title, or put them inside the issue body. I think at a minimum, we should be putting everything in the issue body, but some data should be kept in the title or labels so that people can easily see what an issue is for when scrolling through the issue list.

cincuranet commented 1 year ago

I see as per https://github.com/dotnet/performance/issues/2809, we want to remove the Regression/Improvement labels which should be fine since the title of the issue states whether it tracks regressions or improvements

Not a good idea to remove Regression and Improvement because that's used for filtering during perf triage meeting. And filtering on label is much easier than fiddling with text match.

jeffhandley commented 1 year ago

A couple of different ideas for how to keep the remaining labels as labels:

  1. We could prefix these labels with performance- or perf- or perf-run-

    • perf-regression would not have the potential for confusion with regression-from-last-release and would add the needed context in the destination repos
    • perf-AOT=(true|false), perf-LLVM=(true|false), etc. could still be perceived as noise, but at least they'd all be grouped together and easily understood as data elements included from the run
  2. If we wanted to get fancy and (over-)engineer this, we could write a GitHub app that would transfer the issues to destination repos with some sanitization included in the process

    • Dropping a comment of /transfer to dotnet/runtime could be picked up by the app
    • The app could be configured to know how to map/reduce labels and other data on the issue before the transfer
    • Despite my suggesting it, I think the likelihood of us justifying this to ourselves is extremely low--none of us needs another piece of automation to maintain 🙃
caaavik-msft commented 1 year ago

I'd done a bit of digging into the labels on the runtime repo and found some more that line up and came up with the following mapping:

jeffhandley commented 1 year ago

Note that the area- prefixed labels have special meaning--they cause notifications to team members and imply ownership. We can also only have 1 area- label on any issue to ensure the routing works effectively. While those specific area-CodeGen-X-mono labels might end up being right (if we find the issues to be isolated to those scenarios), I'd defer to the folks involved in the perf issue triage meetings if we would want to actually map that label.

kunalspathak commented 1 year ago

I don't see Ampere label in runtime. As you said, these are specifically to know that the benchmarks were ran on traditional Qualcomm arm machines or Ampere. If we have stopped running on qualcomm (which I think we have), we should just eliminate the Ampere label because we already have arm64 label.

Look Again is something explicitly for issues that we revisit after couple of weeks. I don't think we should map it to something else and should be removed from the issues before transferring although it is very rare that we would transfer the "Look again" issue to runtime repo. Most of the time they are noise.

caaavik-msft commented 1 year ago

We have some runs done on Windows.10.Arm64.Perf.Surf and Windows.10.Amd64.Pixel.Perf queues today which are configured with auto-filing and they are marked as arm64, but not ampere by the autofiling bot. I believe that Windows.10.Arm64.Perf.Surf is Qualcomm, not sure about the Pixel though. Would we still be good to remove the Ampere label?

kunalspathak commented 1 year ago

Would we still be good to remove the Ampere label?

Sometimes, we may need to differentiate if the ARM64 perf issue is Qualcomm or Ampere. would be good to keep it.

caaavik-msft commented 1 year ago

These changes have now been made, however I saw the other issue about fixing the labels getting transferred to runtime is still open, I think it should be reopened if it is still needed since the labels portion has been completed.