Skip to content

Add Bencher to track Benchmarks Performance Regression#3611

Merged
rv-jenkins merged 12 commits intodevelopfrom
add-bencher
Sep 5, 2023
Merged

Add Bencher to track Benchmarks Performance Regression#3611
rv-jenkins merged 12 commits intodevelopfrom
add-bencher

Conversation

@Robertorosmaninho
Copy link
Copy Markdown
Collaborator

Fixes #3517
Follow up of #3603

This PR Introduces Bencher.dev to track performance regressions on our benchmarks.
The Benchmarks already run in the Test-PR Workflow as "Performance Tests" CI. This PR introduces a third-party program that will collect the JSON results of these executions and display them in a graphical online tool.

It will also set a threshold for the time/space that each test, and if an execution passes this limit, the tool will post a comment on the PR warning the developer which tests the regression was observed. The CI will also fail, which won't prevent the PR from being merged.

@rv-jenkins rv-jenkins changed the base branch from master to develop August 30, 2023 17:33
…m sequentially to get confident execution results metrics
@Robertorosmaninho
Copy link
Copy Markdown
Collaborator Author

Add -v to kompile

@Robertorosmaninho Robertorosmaninho marked this pull request as ready for review August 31, 2023 21:29
@Robertorosmaninho Robertorosmaninho requested a review from a team as a code owner August 31, 2023 21:29
Comment thread k-distribution/tests/profiling/pl-tutorial/imp/Makefile
Comment thread .github/workflows/test-pr.yml
Comment thread k-distribution/tests/profiling/pl-tutorial/imp/Makefile
Removing kompile-dir to multiple iterations doesn't use cache;
Removing working around to output verbose execution as Bencher fixed this.
@Robertorosmaninho
Copy link
Copy Markdown
Collaborator Author

Robertorosmaninho commented Sep 4, 2023

@radumereuta I've addressed your comments, and I'm now removing the kompiled directory after each iteration. Although, I think the appropriate solution would be a new frontend flag like --no-cache or --do-not-use-cache.

The flag --profile-rule-parsing indeed does this, but it does more than we want as well, so it's a workaround as much as deleting the directory. I choose the last option as it's more strict for our purpose here.

Copy link
Copy Markdown
Contributor

@radumereuta radumereuta left a comment

Choose a reason for hiding this comment

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

lgtm
Deleting the directory is the safest option. It's too easy to mess something up in the code and get erroneous results.
I would like it if someone else also had a look at the PR.
Great work, though.

Copy link
Copy Markdown
Member

@F-WRunTime F-WRunTime left a comment

Choose a reason for hiding this comment

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

I can't comment on the lower level Makefiles but the changes to the workflow look reasonable

@dwightguth
Copy link
Copy Markdown
Contributor

Looks good, but where is the link to the results?

@Robertorosmaninho
Copy link
Copy Markdown
Collaborator Author

Looks good, but where is the link to the results?

The link for each result is generated after every execution of bencher run on the Performance Tests CI under the Profiling Performance Tests step as:

View results:
- AVM-SEMANTICS (Elapsed Real time (wall clock) in Seconds): https://bencher.dev/console/projects/k-framework/perf?metric_kind=build-time&branches=7fba2f4e-5e87-4782-92a7-9dfc4c8aaa1d&testbeds=d9eea46c-dd6c-4d0e-a830-30581a4e4446&benchmarks=3744f607-9385-4366-a682-5128ce2df25f
- AVM-SEMANTICS (Maximum Resident Set Size in KB): https://bencher.dev/console/projects/k-framework/perf?metric_kind=max-resident-set-size&branches=7fba2f4e-5e87-4782-92a7-9dfc4c8aaa1d&testbeds=d9eea46c-dd6c-4d0e-a830-30581a4e4446&benchmarks=3744f607-9385-4366-a682-5128ce2df25f

.
But for a general purpose, we can define this as the Main Benchmark Page.

@rv-jenkins rv-jenkins merged commit 7f9f5c0 into develop Sep 5, 2023
@rv-jenkins rv-jenkins deleted the add-bencher branch September 5, 2023 20:24
@Baltoli Baltoli mentioned this pull request Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build kompile performance tracker

5 participants