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.

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

Add Action to run performance benchmarks #4285

Merged
merged 39 commits into from
Aug 10, 2023
Merged

Conversation

pmaytak
Copy link
Contributor

@pmaytak pmaytak commented Aug 3, 2023

Completes partially #4237

Changes proposed in this request
Removes test case (cache size 1/1000) because it's redundant, as discussed.
Adds yml to run the benchmarks in the perf test project using a GitHub Action.
Publishes simple results to GitHub pages: https://azuread.github.io/microsoft-authentication-library-for-dotnet/benchmarks/
Breaks if detects 30% regression from last perf run.
Currently only runs for commits to the main branch.

Documentation

  • All relevant documentation is updated.

@pmaytak pmaytak marked this pull request as ready for review August 5, 2023 05:59
@pmaytak pmaytak marked this pull request as ready for review August 5, 2023 06:01
@bgavrilMS
Copy link
Member

I can't see any trace of the commit "211047c" in the history of the main branch, but I found it to be one of the 31 commits in here.

However, am I right to understand that we will only track commits that make it into main (i.e. CI runs)?

@pmaytak
Copy link
Contributor Author

pmaytak commented Aug 7, 2023

I can't see any trace of the commit "211047c" in the history of the main branch, but I found it to be one of the 31 commits in here.

However, am I right to understand that we will only track commits that make it into main (i.e. CI runs)?

The commits in the chart are from my feature branch. This yaml is currently set to run on merge to main. If we set it to run on each commit in the PR, I don't know if that's necessary. It will add noise to the graphs too; although there's an option to exclude these type of runs from updating the graphs, but I wasn't able to make it work.

@pmaytak pmaytak enabled auto-merge (squash) August 10, 2023 03:52
@gladjohn
Copy link
Contributor

I can't see any trace of the commit "211047c" in the history of the main branch, but I found it to be one of the 31 commits in here.
However, am I right to understand that we will only track commits that make it into main (i.e. CI runs)?

The commits in the chart are from my feature branch. This yaml is currently set to run on merge to main. If we set it to run on each commit in the PR, I don't know if that's necessary. It will add noise to the graphs too; although there's an option to exclude these type of runs from updating the graphs, but I wasn't able to make it work.

per this, Do not run this workflow on pull request since this workflow has permission to modify contents.

@pmaytak
Copy link
Contributor Author

pmaytak commented Aug 10, 2023

I can't see any trace of the commit "211047c" in the history of the main branch, but I found it to be one of the 31 commits in here.
However, am I right to understand that we will only track commits that make it into main (i.e. CI runs)?

The commits in the chart are from my feature branch. This yaml is currently set to run on merge to main. If we set it to run on each commit in the PR, I don't know if that's necessary. It will add noise to the graphs too; although there's an option to exclude these type of runs from updating the graphs, but I wasn't able to make it work.

per this, Do not run this workflow on pull request since this workflow has permission to modify contents.

https://github.com/benchmark-action/github-action-benchmark#run-only-on-your-branches

They suggest not running on PR since it will push results to dashboard for each commit for anyone who creates a PR. I wasn't able to exclude the dashboard update in that case. The only option is something like duplicating the whole YAML - one for PR, one for CI with different variables. This can be done later if needed.

@pmaytak pmaytak merged commit 25ccce8 into main Aug 10, 2023
6 checks passed
@pmaytak pmaytak deleted the pmaytak/perf-benchmark-action branch August 10, 2023 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants