Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Automate running benchmarks in PRs that are more likely to impact performance #2606

Closed
1 of 2 tasks
celinval opened this issue Jul 14, 2023 · 3 comments · Fixed by #2837
Closed
1 of 2 tasks

Automate running benchmarks in PRs that are more likely to impact performance #2606

celinval opened this issue Jul 14, 2023 · 3 comments · Fixed by #2837
Assignees
@celinval
Labels
[C] Internal Tracks some internal work. I.e.: Users should not be affected. [I] CI / Infrastructure Work done to CI, tests and infrastructure.

Comments

@celinval
Copy link
Contributor

celinval commented Jul 14, 2023 •
edited
Loading

Proposed change:

  • Automatically run performance benchmark workflow if the PR code has changed and the PR has the "Z-benchCI" label.
  • Automatically tag PRs with "Z-benchCI" if they touch files are likely to impact Kani's performance. For example:
    • kani-compiler/**
    • rust-toolchain.toml
    • kani-dependencies
    • kani-driver/src/call-*

Motivation: After #2604, running benchmark workflow for a PR will be triggered manually. In order to reduce the risks of forgetting to do so, we should automate running the workflow in riskier scenarios.

My suggestion is to use the Z-benchCI as the source of truth for whether the workflow runs or not. This will allow us to override the automation. For example, if the changes only touches comments of a compiler file.
@celinval celinval added [C] Internal Tracks some internal work. I.e.: Users should not be affected. [I] CI / Infrastructure Work done to CI, tests and infrastructure. labels Jul 14, 2023
@celinval
Copy link
Contributor Author

See https://github.com/marketplace/actions/labeler for that

@celinval celinval mentioned this issue Sep 21, 2023
Merged
4 tasks
@celinval celinval self-assigned this Sep 26, 2023
@celinval
Copy link
Contributor Author

It looks like the auto label logic doesn't work to trigger other github actions. See: https://github.com/orgs/community/discussions/33804

@celinval
Copy link
Contributor Author

celinval commented Oct 5, 2023

Just to clarify what's going on. The github documentation has a good explanation on why the auto label workflow does not trigger a 'labeled' action:

When you use the repository's GITHUB_TOKEN to perform tasks, events triggered by the GITHUB_TOKEN, [redacted] , will not create a new workflow run.

To fix this issue, we have the following options:

  • Create a new token and use that instead.
  • Explicitly trigger the workflow when the label is added.

@celinval celinval mentioned this issue Oct 6, 2023
Closed
@celinval celinval mentioned this issue Oct 25, 2023
Merged
celinval added a commit that referenced this issue Oct 26, 2023
@celinval
Create workflow Kani Extra for checks that run conditionally on labels (
ecd8957 
#2837)

The "Auto Label" workflow correctly label our PRs; however, this action
do not trigger a PR 'labeled' workflow. To automatically do that, we now
changed the CI structure to have a new workflow with extra jobs that are
conditional to labels and files being modified.

This new workflow starts by triggering the auto-label job. The benchmark
workflow is only triggered if after the auto-label job, the PR has a
Z-BenchCI label.

This also means that the benchmark workflow will now run after any
update to a PR that has the 'Z-BenchCI' label.

Resolves #2606
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@celinval celinval

Labels
[C] Internal Tracks some internal work. I.e.: Users should not be affected. [I] CI / Infrastructure Work done to CI, tests and infrastructure.
Projects
No open projects
Kani 2023-10-03
Status: No status
Milestone
No milestone
1 participant
@celinval