Skip to content

Continuous benchmarking in CI #7100

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

Merged
merged 13 commits into from
Oct 13, 2024
Merged

Continuous benchmarking in CI #7100

merged 13 commits into from
Oct 13, 2024

Conversation

cknitt
Copy link
Member

@cknitt cknitt commented Oct 12, 2024

This PR runs the syntax benchmark in CI and uses https://github.com/benchmark-action/github-action-benchmark to have the github-actions bot create a comment with the benchmark results in future PRs. It will also warn if the results diverge too much from the previous ones (currently 150% is set as the alerting threshold, we can fine tune this later).

It cleans up Benchmark.ml and modifies it to output JSON for consumption by github-action-benchmark.

It also fixes two bugs in the benchmark logic:

  • Incorrect implementation of caml_mach_absolute_time for Linux
  • The number of allocations for one run was divided by the number of runs, instead of the total number of allocations.

Also note that:

  • The comment that the bot left in this PR is outdated. It seems it only comments once on PR creation.
  • For permission reasons, the bot commenting in the PR will only work for PRs created from branches in the compiler repo, not from branches in forks.

@cknitt cknitt changed the title Benchmark Continuous benchmarking in CI Oct 12, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 13163ed Previous: 8e0c826 Ratio
Parse RedBlackTree.res - avg. time 0.00000126 ms 0.0000011733333333333333 ms 1.07
Parse RedBlackTree.res - allocations 1327 1327 1
Parse RedBlackTree.res - bytes allocated 10616 10616 1
Print RedBlackTree.res - avg. time 0.0000023933333333333334 ms 0.000002353333333333333 ms 1.02
Print RedBlackTree.res - allocations 1576 1576 1
Print RedBlackTree.res - bytes allocated 12608 12608 1
Print RedBlackTreeNoComments.res - avg. time 0.0000021733333333333334 ms 0.0000021866666666666668 ms 0.99
Print RedBlackTreeNoComments.res - allocations 1661 1661 1
Print RedBlackTreeNoComments.res - bytes allocated 13289 13289 1
Parse Napkinscript.res - avg. time 0.000038826666666666665 ms 0.00003814 ms 1.02
Parse Napkinscript.res - allocations 58176 58176 1
Parse Napkinscript.res - bytes allocated 465413 465413 1
Print Napkinscript.res - avg. time 0.00007534666666666666 ms 0.00007474666666666666 ms 1.01
Print Napkinscript.res - allocations 65175 65175 1
Print Napkinscript.res - bytes allocated 521404 521404 1
Parse HeroGraphic.res - avg. time 0.000005286666666666666 ms 0.00000502 ms 1.05
Parse HeroGraphic.res - allocations 8128 8128 1
Parse HeroGraphic.res - bytes allocated 65030 65030 1
Print HeroGraphic.res - avg. time 0.000008893333333333333 ms 0.000008806666666666666 ms 1.01
Print HeroGraphic.res - allocations 9309 9309 1
Print HeroGraphic.res - bytes allocated 74477 74477 1

This comment was automatically generated by workflow using github-action-benchmark.

@cknitt cknitt marked this pull request as ready for review October 13, 2024 08:47
@cknitt cknitt requested review from zth and cristianoc October 13, 2024 08:47
@cknitt
Copy link
Member Author

cknitt commented Oct 13, 2024

  • The comment that the bot left in this PR is outdated. It seems it only comments once on PR creation.

Current output looks like this:

[
{"name":"Parse RedBlackTree.res - time/run","unit":"ms","value":0.7873361333333333},
{"name":"Parse RedBlackTree.res - allocs/run","unit":"words","value":199154},
{"name":"Print RedBlackTree.res - time/run","unit":"ms","value":1.3649105666666665},
{"name":"Print RedBlackTree.res - allocs/run","unit":"words","value":232111},
{"name":"Print RedBlackTreeNoComments.res - time/run","unit":"ms","value":1.2760960733333333},
{"name":"Print RedBlackTreeNoComments.res - allocs/run","unit":"words","value":246137},
{"name":"Parse Napkinscript.res - time/run","unit":"ms","value":22.582111926666666},
{"name":"Parse Napkinscript.res - allocs/run","unit":"words","value":8669132},
{"name":"Print Napkinscript.res - time/run","unit":"ms","value":43.12553384666666},
{"name":"Print Napkinscript.res - allocs/run","unit":"words","value":10038503},
{"name":"Parse HeroGraphic.res - time/run","unit":"ms","value":3.1373524666666666},
{"name":"Parse HeroGraphic.res - allocs/run","unit":"words","value":1219330},
{"name":"Print HeroGraphic.res - time/run","unit":"ms","value":4.988513046666666},
{"name":"Print HeroGraphic.res - allocs/run","unit":"words","value":1396464}
]

Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

Sweet!
Is there a way to always comment on the difference w.r.t. baseline even if it's within threshold of not causing an alarm?

@cknitt
Copy link
Member Author

cknitt commented Oct 13, 2024

Is there a way to always comment on the difference w.r.t. baseline even if it's within threshold of not causing an alarm?

Yes, the option

comment-always: true

should do that.

@cknitt cknitt merged commit e1b7fb7 into master Oct 13, 2024
20 checks passed
@cknitt cknitt deleted the benchmark branch October 13, 2024 16:48
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.

2 participants