Repository navigation
doc: state the thresholds that CI's performance gate uses - #981
Merged
Merged
Conversation
#400 replaced the thresholds in doc/benchmarks.md, 50% and 10 ms, with the defaults of performance gate, 40% and 5 ms. That paragraph describes the CI gate, and ci.yml has passed 50% and 10 ms since the first commit, so the guide described a stricter gate than CI runs. Restore CI's thresholds, name ci.yml as their source, and point to the command's defaults in the CLI reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The benchmark guide states the thresholds that ci.yml passes to the regression gate, and both guides state the defaults of performance gate. Read both from their sources, so that a change to either, or an edit like #400's, fails the tests until the guides follow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the benchmark threshold documentation correction and accompanying regression test.
No open issues 🎉
What changed in this PR
This PR corrects the benchmark guide to distinguish CI’s performance-gate thresholds from the CLI defaults. It also adds a regression test to keep the documentation aligned with both sources.
Changes:
- Documents CI’s 50% regression threshold and 10 ms absolute tolerance, while preserving the CLI defaults of 40% and 5 ms.
- Checks the documented values against the CI workflow and CLI parser defaults.
| File | Description |
|---|---|
scripts/test_performance.py |
Adds consistency checks for thresholds documented in the benchmark and usage guides. |
doc/benchmarks.md |
Corrects CI thresholds and links to the workflow and CLI defaults. |
Review effort: Balanced
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.
Summary
doc/benchmarks.mdsays the regression gate fails a metric that is more than 40% worse and, for millisecond latencies, also more than 5 ms slower. CI passes 50% and 10 ms (ci.yml), as it has since the first commit. #400 set the guide to the defaults ofperformance gate, 40% and 5 ms, but that paragraph describes the CI gate.doc: state the thresholds that CI's performance gate usesci.ymlas their source, and points to the command's defaults in the CLI reference.scripts: check that the guides state the performance gate's thresholdsci.ymland the defaults from theperformance gateparser, and checks thatdoc/benchmarks.mdanddoc/usage.mdstate them.Found while investigating #415.
Validation
Windows 11, Python 3.14:
python -m unittest scripts/test_performance.py: 100 tests pass.doc/benchmarks.md:'more than 50% worse' not found.ruff check,ruff format --check,pyright --pythonplatform LinuxandWindows, andscripts/check_markdown_links.pypass.classify-ci-changesselects no platform, microVM, or OpenVMM suite for these two files. This run builds the guest artifacts and OpenVMM and runs the platform benchmarks anyway: the change detector diffs the base and head commits directly, and this branch is onedata:baseline commit behinddev.