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: Continuous Benchmarking #166

Closed
jserv opened this issue Jul 19, 2023 · 6 comments · Fixed by #213
Closed

CI: Continuous Benchmarking #166

jserv opened this issue Jul 19, 2023 · 6 comments · Fixed by #213
Assignees

Comments

@jserv
Copy link
Contributor

jserv commented Jul 19, 2023

github-action-benchmark provides a GitHub Action for continuous benchmarking, and we can utilize it to track performance regressions.

Expected output:

  • Integrate github-action-benchmark in CI pipeline.
  • Deploy self-hosted runners for stable benchmarking environments. Arm64 and x86-64 hosts included.
  • Ensure that all benchmark charts from above workflows are gathered in GitHub pages.

Reference:

@ChinYikMing
Copy link
Collaborator

Support pull requests. Instead of updating GitHub pages, add a comment to the pull request to explain benchmark results.

Quote from github-action-benchmark's future work.

If this CI run is enabled for PRs, the benchmark charts will update with each pull request. Since we must first confirm a performance improvement before updating the benchmark charts, this might not be what we desire. In other words, we need to discuss about how the PR is doing before updating the benchmark chart. Or, is it acceptable to update the benchmark chart with each PR?

And one more thing, may I know if @qwe661234 is working on this issue? If not, I could help to write the CI script if the above concern is OK.

@jserv
Copy link
Contributor Author

jserv commented Aug 29, 2023

And one more thing, may I know if @qwe661234 is working on this issue? If not, I could help to write the CI script if the above concern is OK.

Please move forward. Before submitting pull request(s), do write down your plans and communicate here.

@jserv jserv assigned ChinYikMing and unassigned qwe661234 and ChinYikMing Aug 29, 2023
@jserv
Copy link
Contributor Author

jserv commented Aug 29, 2023

If this CI run is enabled for PRs, the benchmark charts will update with each pull request. Since we must first confirm a performance improvement before updating the benchmark charts, this might not be what we desire. In other words, we need to discuss about how the PR is doing before updating the benchmark chart. Or, is it acceptable to update the benchmark chart with each PR?

If the results generated by the CI are deterministic, we can certainly treat them as references for incremental development. However, due to our current lack of confidence, it is premature to define a clear strategy at this stage.

@ChinYikMing
Copy link
Collaborator

ChinYikMing commented Sep 2, 2023

My plan is as follows:

  1. Run the benchmark workflow and update the GitHub pages whenever "pushing on master branch" and "opening of a pull request and synchronizing the pull request(if any) on master branch" are performed when editing the "emulate.c" and "rv32_template.c" files because these 2 files have the potential to directly impact the benchmark. Under this constraint, we can prohibit excessive benchmark workflow with unrelated files, such as those in docs/ . Do you have any suggestions for other files that might impact the benchmark?

  2. The benchmark comparison between the most recent commit and the previous commit will be noted, allowing the maintainer and committer to debate the benchmark comparison. An example of a benchmark comparison can be seen in the benchmark action README: link

  3. Main benchmark will be “Dhrystone” and “CoreMark” in tests/ for now because I can easily modify the script to generate the JSON format file which required by the benchmark action.

  4. The x-axis and y-axis unit of benchmark graph will be commit SHA and unit of benchmark respectively. Let’s take “Dhrystone” be an example: the y-axis will be average MIPS over 10 runs. Example benchmark graph taken from the benchmark action README: link

@ChinYikMing
Copy link
Collaborator

ChinYikMing commented Sep 3, 2023

  1. The benchmark comparison between the most recent commit and the previous commit will be noted, allowing the maintainer and committer to debate the benchmark comparison. An example of a benchmark comparison can be seen in the benchmark action README: link

We can set the threshold to trigger the generation of benchmark comparison, for example 0.5x worse than before, default is 2x worse than before.

@jserv
Copy link
Contributor Author

jserv commented Sep 6, 2023

  1. Run the benchmark workflow and update the GitHub pages whenever "pushing on master branch" and "opening of a pull request and synchronizing the pull request(if any) on master branch" are performed when editing the "emulate.c" and "rv32_template.c" files because these 2 files have the potential to directly impact the benchmark. Under this constraint, we can prohibit excessive benchmark workflow with unrelated files, such as those in docs/ . Do you have any suggestions for other files that might impact the benchmark?

We shall track decode.c as well.

  1. Main benchmark will be “Dhrystone” and “CoreMark” in tests/ for now because I can easily modify the script to generate the JSON format file which required by the benchmark action.

Agree.

jserv pushed a commit that referenced this issue Sep 11, 2023
The 'pull_request_target' event is utilized for the benchmark action due
to its ability to update data in the 'gh-pages' branch, which is crucial
for visualization on GitHub Pages. The 'pull_request' event lacks the
necessary 'GITHUB_TOKEN' for this task.

Furthermore, 'workflow_dispatch' event is added to enable the initial
setup and running of the benchmark. This permits the storage of base
benchmark data for future comparisons.

To prevent redundant executions, a filter has been implemented to
exclude the merging push event.

Close #166
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 a pull request may close this issue.

3 participants