Merged
Conversation
celinval
reviewed
Mar 8, 2023
Contributor
celinval
left a comment
There was a problem hiding this comment.
Thank you. This looks very clean and organized. I like how you added a bunch of README.md everywhere. This is something that we should be doing more often!
f4770bf to
02fc6b2
Compare
zhassan-aws
approved these changes
Mar 15, 2023
Contributor
There was a problem hiding this comment.
Looks great @karkhaz! I have a few comments, mostly minor.
This commit adds a new tool called `benchcomp` to kani. `benchcomp` is a tool for comparing one or more suites of benchmarks using two or more 'variants' (command line arguments and environment variables). `benchcomp` runs all combinations of suite x variant, parsing the unique output formats of each of these runs. `benchcomp` then combines the parsed outputs and writes them into a single file. `benchcomp` can post-process that combined file to create visualizations, exit if the results are not as expected, or perform other actions. All subcommands of `benchcomp` are noops in this commit; future commits will fill in the implementation.
This commit adds a single unit test that validates a simple config file against a schema. The script that runs the tests is currently quite minimal, but will be expanded to support regression tests in a future commit.
`benchmark run` is a command that runs every combination of suite x variant, and writes their results out to files in a single directory. `benchcomp collate` reads all the files from that directory and writes out a single combined file that can be further post-processed. This commit also adds a regression test, together with regression test infrastructure. The commit also adds a trivial 'parser' for the test.
70aa2d0 to
394dc86
Compare
This was referenced Mar 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
This PR adds benchcomp, a tool for comparing one or more suites of benchmarks using two or more 'variants' (command line arguments and environment variables).
benchcompruns all combinations of suite x variant, parsing the unique output formats of each of these runs.benchcompthen combines the parsed outputs and writes them into a single file.benchcompcan post-process that combined file to create visualizations, exit if the results are not as expected, or perform other actions.Testing:
This PR currently has a single unit test that validates a configuration file, and a single regression test that runs
benchcomp runandbenchcomp collatetogether. The tests are currently minimal because this PR does not include a realistic parser for benchmark output. The included trivial parser is enough to exercise the two currentbenchcompsubcommands, but more substantial tests will be added when a real parser gets included.Checklist
This PR fixes #2273.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.