Skip to content

ci: Releasing benchmarks and benchmarking PR#2422

Closed
JaydipGabani wants to merge 3 commits into
open-policy-agent:masterfrom
JaydipGabani:master
Closed

ci: Releasing benchmarks and benchmarking PR#2422
JaydipGabani wants to merge 3 commits into
open-policy-agent:masterfrom
JaydipGabani:master

Conversation

@JaydipGabani
Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
Releasing benchmark stats with each release and comparing benchmark data from each PR against latest released benchmark stats.

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #692

Special notes for your reviewer:

@sozercan sozercan changed the title chore: Releasing benchmarks and benchmarking PR ci: Releasing benchmarks and benchmarking PR Dec 1, 2022
@JaydipGabani JaydipGabani force-pushed the master branch 5 times, most recently from ec42591 to 635fe99 Compare December 1, 2022 02:34
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 1, 2022

Codecov Report

Base: 53.71% // Head: 53.61% // Decreases project coverage by -0.09% ⚠️

Coverage data is based on head (6073a59) compared to base (36ddb2f).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2422      +/-   ##
==========================================
- Coverage   53.71%   53.61%   -0.10%     
==========================================
  Files         117      117              
  Lines       10281    10281              
==========================================
- Hits         5522     5512      -10     
- Misses       4341     4347       +6     
- Partials      418      422       +4     
Flag Coverage Δ
unittests 53.61% <ø> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...onstrainttemplate/constrainttemplate_controller.go 56.93% <0.00%> (-1.92%) ⬇️
pkg/readiness/object_tracker.go 82.91% <0.00%> (-1.07%) ⬇️
pkg/readiness/ready_tracker.go 69.83% <0.00%> (-0.51%) ⬇️
pkg/readiness/list.go 91.17% <0.00%> (+11.76%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JaydipGabani
Copy link
Copy Markdown
Contributor Author

Benchmark job result is located here

Comment thread .github/workflows/workflow.yaml
- name: Download release benchmark file
uses: robinraju/release-downloader@v1.6
id: get-latest-benchmark
continue-on-error: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we want to continue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We are not really executing rest of the steps if the file is missing.

If we dont continue on error, the whole job fails and I think it be best if we not fail on missing previous benchmark file, just skip all the steps of the workflow if release benchmark is missing

Comment thread .github/workflows/workflow.yaml Outdated
id: get-latest-benchmark
continue-on-error: true
with:
latest: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this going to take latest semver or latest release?

Copy link
Copy Markdown
Contributor Author

@JaydipGabani JaydipGabani Dec 3, 2022

Choose a reason for hiding this comment

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

This is going to take latest tagged release

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find an action that returns the latest semver. Still looking, I think we could sort it also after retrieving every release if this is needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we use the github api to get recent releases then find the latest semver?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yip! It took some time, but I was able to get the latest release semver version by calling API and sorting

@JaydipGabani JaydipGabani force-pushed the master branch 3 times, most recently from 4a6a9ba to 55ea1bf Compare December 3, 2022 23:52
@JaydipGabani JaydipGabani requested a review from sozercan December 5, 2022 18:25
export GOPATH="$HOME/go"
PATH="$GOPATH/bin:$PATH"
go install golang.org/x/perf/cmd/benchstat@latest
benchstat release_benchmarks.txt pr_benchmarks.txt > benchstat.txt
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what happens if release_benchmarks.txt doesn't exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If the release_benchmark.txt doesn't exist steps.get-latest-benchmark.outcome results in failure instead of success and hence we skip the rest of the steps.

Comment thread .github/workflows/workflow.yaml Outdated
with:
issue-number: ${{ github.event.pull_request.number }}
body: |
Here is how this PR compares againt latest released version in terms of benchmark. Unless this PR containts huge updates, if the performace is significantly lower consider optimizing your changes to improve the performace closest to latest released benchmark.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Here is how this PR compares againt latest released version in terms of benchmark. Unless this PR containts huge updates, if the performace is significantly lower consider optimizing your changes to improve the performace closest to latest released benchmark.
This PR compares its performance to the latest released version. If it performs significantly lower, consider optimizing your changes to improve the performance.

sozercan and others added 2 commits December 5, 2022 23:58
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
@JaydipGabani JaydipGabani force-pushed the master branch 7 times, most recently from 2e6118a to c0ccd31 Compare December 6, 2022 05:22
@JaydipGabani JaydipGabani force-pushed the master branch 17 times, most recently from 788ff01 to 6073a59 Compare December 6, 2022 06:23
@JaydipGabani JaydipGabani requested a review from sozercan December 6, 2022 17:56
@JaydipGabani JaydipGabani force-pushed the master branch 8 times, most recently from b3dde7e to a9c3998 Compare December 6, 2022 20:03
Signed-off-by: Jaydip Gabani <gabanijaydip@gmail.com>
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.

Show benchmark delta for each PR

3 participants