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

ci: Add benchmark performance comparisons #232

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Feb 25, 2022

DESCRIPTION

This PR modifies the CI flow as follows:

  • In addition to skipping the benchmarking when the action/no-benchmark label is present, now if the PR is going into a branch that is NOT develop branch (base reference != develop) then the benchmarks will not run.
  • In the case that PR is opened and pushes to the PR are happening (synchronize event) or if a new PR is opened (where in both the cases the PR's base branch is develop) and the benchmarks are supposed to run, then CI will go fetch the latest artifact from the most recent build on develop that passes the lint-then-benchmark.yml workflow (ensures artifact was uploaded) and run the benchmark comparisons using benchstat.

ISSUE

Closes #212


Rough Notes On Tools Explored:

  • benchstat: is really simple and we can use built in github action cache to store the previous bench stats (rather than their way) and just run the comparisons and see the results through the CI run workflow (not fun to look at).

  • github-action-benchmark and gobenchdata: both are very similar but the github action benchmark gives you ability to have the benchmarks publish to lets say the docs folder inside the master branch. However output of gobench seems much cleaner (one downside is that we can't really or rather should not run this on PR because it will modify a branch).

  • gobencher: I enabled this github app to generate PR results the tackle the problem of not being able to run gobenchdata or github-action-benchmark on PRs. This creates a very nice comparison on the PR check itself. One down side is that we can't control this with labels

@shahzadlone shahzadlone self-assigned this Feb 25, 2022
@jsimnz jsimnz added area/testing Related to any test or testing suite perf Performance issue or suggestion labels Mar 3, 2022
@jsimnz jsimnz added this to the DefraDB v0.2.1 milestone Mar 3, 2022
@shahzadlone shahzadlone force-pushed the lone/ci/benchmark-comparisons branch from 64967dd to 6f50ebc Compare March 18, 2022 08:33
github.event_name == 'pull_request' &&
github.base_ref == 'develop'
run: >
make deps:bench &&
Copy link
Member Author

@shahzadlone shahzadlone Mar 18, 2022

Choose a reason for hiding this comment

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

This make deps:bench step will be removed once I modify the AMI image on AWS that is used to create the EC2 instance to contain this dependency from the get go.

@shahzadlone shahzadlone marked this pull request as ready for review March 18, 2022 09:25
@shahzadlone shahzadlone requested a review from jsimnz March 18, 2022 09:25
@shahzadlone shahzadlone linked an issue Mar 18, 2022 that may be closed by this pull request
@shahzadlone shahzadlone added the ci/build This is issue is about the build or CI system, and the administration of it. label Mar 18, 2022
@AndrewSisley
Copy link
Contributor

AndrewSisley commented Mar 18, 2022

now if the PR is going into a branch that is NOT develop branch (base reference != develop) then the benchmarks will not run.

It might be nice to have them run when the target is main too, gives a nice summary of the release cycle's impact on users. If it is not a quick thing to add this can added some other time perhaps

EDIT: it looks like this would be a quick or check on two lines in the yaml, but I might be wrong - would suggest doing now

@shahzadlone shahzadlone merged commit 728fa82 into develop Mar 18, 2022
@shahzadlone shahzadlone deleted the lone/ci/benchmark-comparisons branch March 18, 2022 22:18
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
* Implement benchmark comparisons using the last successfully uploaded
benchmark report on the develop branch

* See if GOPATH helps find the downloaded binary of benchstat

* squeeze in one line to see if works now.

* Add `-alpha 1.1` to benchstat to trick it to display the deltas
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Related to any test or testing suite ci/build This is issue is about the build or CI system, and the administration of it. perf Performance issue or suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add benchmark auto reporting/comparison
3 participants