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 github benchmark action #78

Merged

Conversation

mkannwischer
Copy link
Contributor

@mkannwischer mkannwischer commented Jun 26, 2024

This adds benchmark visualization using https://github.com/benchmark-action/github-action-benchmark.

When ever a new commit is pushed to the main branch, it will upload the benchmarks to the https://github.com/pq-code-package/mlkem-c-aarch64/tree/gh-pages branch and they will be visualized at https://pq-code-package.github.io/mlkem-c-aarch64/dev/bench.

This won't run on PRs (and it can't because these workflow runs won't have the permission to push commits -- see my comment below), so we can only see if it works the way it is intended if we merge it.

I have performed extensive tests here: https://github.com/mkannwischer/bench-test
And you can see how it should look like here: https://mkannwischer.github.io/bench-test/dev/bench/

@mkannwischer mkannwischer force-pushed the github-action-benchmark branch from a780699 to 3d53819 Compare June 26, 2024 04:02
@mkannwischer mkannwischer added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Jun 26, 2024
@mkannwischer mkannwischer force-pushed the github-action-benchmark branch from 749b806 to dca1876 Compare June 26, 2024 07:23
@mkannwischer mkannwischer added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Jun 26, 2024
scripts/tests Outdated Show resolved Hide resolved
@mkannwischer mkannwischer added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Jun 26, 2024
@mkannwischer mkannwischer force-pushed the github-action-benchmark branch from 2fd2773 to 61f6ce3 Compare June 26, 2024 08:59
@mkannwischer mkannwischer added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Jun 26, 2024
@mkannwischer mkannwischer added the benchmark this PR should be benchmarked in CI label Jun 26, 2024
Signed-off-by: Matthias J. Kannwischer <[email protected]>
@mkannwischer mkannwischer force-pushed the github-action-benchmark branch from 61f6ce3 to 33ccd2a Compare June 26, 2024 09:19
@potsrevennil potsrevennil added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Jun 26, 2024
@mkannwischer
Copy link
Contributor Author

mkannwischer commented Jun 26, 2024

After lots of experimenting, I figured out that we cannot test this in a pull request.

The maximum permission of the GITHUB_TOKEN in pull requests from forks on contents in the repo is read, but we need write to be able to create commits in the gh-pages branch. In the end we want to only run it on the main branch, so that's not a problem in the end.

See: https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token

I have performed quite some tests in https://github.com/mkannwischer/bench-test today, and I believe that what we have here right now should work.

@potsrevennil potsrevennil removed the benchmark this PR should be benchmarked in CI label Jun 26, 2024
@mkannwischer mkannwischer marked this pull request as ready for review June 26, 2024 09:58
@mkannwischer mkannwischer requested a review from a team June 26, 2024 09:58
Signed-off-by: Matthias J. Kannwischer <[email protected]>
Copy link
Contributor

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments, otherwise LGTM

Signed-off-by: Matthias J. Kannwischer <[email protected]>
Signed-off-by: Matthias J. Kannwischer <[email protected]>
@hanno-becker hanno-becker merged commit de9203e into pq-code-package:main Jun 26, 2024
20 checks passed
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.

3 participants