chore: upgrade performance benchmark#2816
Open
AlbinaBlazhko17 wants to merge 12 commits into
Open
Conversation
|
Contributor
|
Contributor
Coverage Report
File CoverageNo changed files found. |
AlbinaBlazhko17
added
the
performance-benchmark
Contributor
|
AlbinaBlazhko17
added
performance-benchmark
Contributor
|
AlbinaBlazhko17
added
performance-benchmark
Contributor
|
Contributor
|
AlbinaBlazhko17
added
performance-benchmark
Contributor
Performance BenchmarkBundle
Lint
|
AlbinaBlazhko17
added
performance-benchmark
Contributor
Performance BenchmarkBundle
Lint
|
Contributor
Performance BenchmarkBundle
Lint
|
AlbinaBlazhko17
changed the title
kanoru3101
approved these changes
May 27, 2026
| LINT_CMDS=$(cat package.json | jq '.dependencies' | jq 'keys' | jq 'map("'\''node node_modules/" + . + "/bin/cli.js lint all@latest --config=api-definitions/redocly.yaml --generate-ignore-file'\''")' | jq 'join(" ")' | xargs) | ||
|
|
||
| # Lint --prepare wipes the ignore file each run so iterations do equal work: | ||
| echo "REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 2 ${BUNDLE_CMDS} --export-markdown benchmark_bundle.md --export-json benchmark_bundle.json && REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 2 --prepare 'rm -f api-definitions/.redocly.lint-ignore.yaml' ${LINT_CMDS} --export-markdown benchmark_lint.md --export-json benchmark_lint.json" > test-command.txt |
Contributor
There was a problem hiding this comment.
&& skips lint if bundle fails on any version. Intentional? ; would keep them independent.
Contributor
Author
There was a problem hiding this comment.
Good point, but if bundle files we need to immediately fix the issue and then re-run a benchmark.
| LINT_CMDS=$(cat package.json | jq '.dependencies' | jq 'keys' | jq 'map("'\''node node_modules/" + . + "/bin/cli.js lint all@latest --config=api-definitions/redocly.yaml --generate-ignore-file'\''")' | jq 'join(" ")' | xargs) | ||
|
|
||
| # Lint --prepare wipes the ignore file each run so iterations do equal work: | ||
| echo "REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 2 ${BUNDLE_CMDS} --export-markdown benchmark_bundle.md --export-json benchmark_bundle.json && REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 2 --prepare 'rm -f api-definitions/.redocly.lint-ignore.yaml' ${LINT_CMDS} --export-markdown benchmark_lint.md --export-json benchmark_lint.json" > test-command.txt |
Contributor
There was a problem hiding this comment.
Can you add a comment on why we use warmup=2
tatomyr
reviewed
May 28, 2026
| # `labeled` lets applying the `performance-benchmark` label trigger a rerun | ||
| # without needing an empty commit; the job-level `if` filters out unrelated | ||
| # label changes so we don't burn CI on every label add. | ||
| types: [opened, synchronize, reopened, labeled] |
Collaborator
There was a problem hiding this comment.
I don't really think we need to rerun benchmarks too often. Most of the time the current flow is sufficient to see if there's a noticeable difference in perf. If you decide to leave the labeling though, please remove the comments as they explain a trivial case.
Contributor
Author
There was a problem hiding this comment.
Changes in workflow was introduced only for testing purposes for this PR, i will remove this after PR will be approved. I left note in the description of PR regarding it.
| if: github.head_ref == 'changeset-release/main' | ||
| # Runs on the release PR (changeset-release/main) automatically, or on | ||
| # any PR carrying the `performance-benchmark` label for opt-in deep-dive. | ||
| if: github.head_ref == 'changeset-release/main' || contains(github.event.pull_request.labels.*.name, 'performance-benchmark') |
Collaborator
There was a problem hiding this comment.
Again, I'm not sure how useful is to run the historical version benchmarks on individual PRs.
| with: | ||
| file-path: tests/performance/benchmark_chart.md | ||
| comment-tag: historical-versions-comparison | ||
| comment-tag: ${{ contains(github.event.pull_request.labels.*.name, 'performance-benchmark') && format('historical-versions-rerun-{0}', github.run_id) || 'historical-versions-comparison' }} |
Collaborator
There was a problem hiding this comment.
This must be needless if we remove the labeling approach. Am I right?
| return sorted.length % 2 ? sorted[mid] : (sorted[mid - 1] + sorted[mid]) / 2; | ||
| }; | ||
|
|
||
| const medianAbsoluteDeviation = (xs, centre) => median(xs.map((x) => Math.abs(x - centre))); |
Collaborator
There was a problem hiding this comment.
Doesn't hyperfine calculate the deviation already, does it?
Contributor
Author
There was a problem hiding this comment.
Good point, i give it a try and found that hyperfine doesn't have MAD, only stddev.
| "test:lint": "REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 3 'node node_modules/cli-latest/bin/cli.js lint all@latest --config=api-definitions/redocly.yaml --generate-ignore-file' 'node node_modules/cli-next/bin/cli.js lint all@latest --config=api-definitions/redocly.yaml --generate-ignore-file'", | ||
| "test:check-config": "REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 3 'node node_modules/cli-latest/bin/cli.js check-config --config=api-definitions/redocly.yaml --lint-config=warn' 'node node_modules/cli-next/bin/cli.js check-config --config=api-definitions/redocly.yaml --lint-config=warn'" | ||
| "test:bundle": "REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 2 'node node_modules/cli-latest/bin/cli.js bundle all@latest --config=api-definitions/redocly.yaml' 'node node_modules/cli-next/bin/cli.js bundle all@latest --config=api-definitions/redocly.yaml'", | ||
| "test:lint": "REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 2 --prepare 'rm -f api-definitions/.redocly.lint-ignore.yaml' 'node node_modules/cli-latest/bin/cli.js lint all@latest --config=api-definitions/redocly.yaml --generate-ignore-file' 'node node_modules/cli-next/bin/cli.js lint all@latest --config=api-definitions/redocly.yaml --generate-ignore-file'", |
Collaborator
There was a problem hiding this comment.
Why remove rm -f api-definitions/.redocly.lint-ignore.yaml?
Contributor
Author
There was a problem hiding this comment.
Initially i have two approaches regarding generating lint-ignore file:
- Not to remove lint ignore file before every test run. In this case every time we will change existing file, which was generate in the first run. In my opinion it can produce artifacts in benchmark, because file size will be increased during testing due to various CLI versions can have different rules. As we want to be more accurate in benchmarks i discarded this variant.
- Remove previous lint ignore file and generate new one in every run. I pick this approach
| "scripts": { | ||
| "chart": "node chart.js > benchmark_chart.md", | ||
| "make-test": "bash make-test-command.sh", | ||
| "test:bundle": "REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 3 'node node_modules/cli-latest/bin/cli.js bundle all@latest --config=api-definitions/redocly.yaml' 'node node_modules/cli-next/bin/cli.js bundle all@latest --config=api-definitions/redocly.yaml'", |
Collaborator
There was a problem hiding this comment.
This wasn't supposed to be committed 🙁.
Contributor
Author
There was a problem hiding this comment.
I changed warmup 3 to 2, because we discussed, that initially we have warmup 1 and it will produce invalid results from time to time. Before my PR someone change this to 3. I decided to put 2 as the median and as we have additional lint command running we need to decrease time of benchmark run.
Comment on lines
+9
to
+10
| BUNDLE_CMDS=$(cat package.json | jq '.dependencies' | jq 'keys' | jq 'map("'\''node node_modules/" + . + "/bin/cli.js bundle all@latest --config=api-definitions/redocly.yaml'\''")' | jq 'join(" ")' | xargs) | ||
| LINT_CMDS=$(cat package.json | jq '.dependencies' | jq 'keys' | jq 'map("'\''node node_modules/" + . + "/bin/cli.js lint all@latest --config=api-definitions/redocly.yaml --generate-ignore-file'\''")' | jq 'join(" ")' | xargs) |
Collaborator
There was a problem hiding this comment.
Let's extract the common part and put specific options to line 13.
Contributor
Author
There was a problem hiding this comment.
I did this. Thanks, looks better.
Contributor
Performance BenchmarkBundle
Lint
|
Contributor
Performance BenchmarkBundle
Lint
|
tatomyr
reviewed
May 28, 2026
| # Store the command into a text file: | ||
| echo REDOCLY_SUPPRESS_UPDATE_NOTICE=true hyperfine --warmup 1 $(cat package.json | jq '.dependencies' | jq 'keys' | jq 'map("'\''node node_modules/" + . + "/bin/cli.js bundle all@latest --config=api-definitions/redocly.yaml'\''")' | jq 'join(" ")' | xargs) --export-markdown benchmark_check.md --export-json benchmark_check.json > test-command.txt | ||
| build_cmds() { | ||
| jq -r --arg sub "$1" --arg extra "${2:-}" '.dependencies | keys | map("'\''node node_modules/" + . + "/bin/cli.js " + $sub + " all@latest --config=api-definitions/redocly.yaml" + $extra + "'\''") | join(" ")' package.json |
Collaborator
There was a problem hiding this comment.
Let's discuss how to make it simpler.
tatomyr
reviewed
May 28, 2026
Comment on lines
+20
to
+21
| const json = JSON.parse(fs.readFileSync(jsonPath, 'utf8')); | ||
| const rows = json.results.map((r) => [ |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9a61a65. Configure here.
Contributor
Performance Benchmark
|
Contributor
Performance Benchmark
|
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
3 participants
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.
What/Why/How?
Mean+StddevtoMedian+MAD,because sometimes measurement in CI can produce a huge glitch. e.g. we measure: 1.01, 1.02, 1.02, 6.4. In case when we haveMeanwe will calculate sum divided by number, so this glitch will have a huge influence on a final number, but for theMedianwe will not have this.2.0.0 - left as starting point
2.11.1 - left for testing, no perf changes
2.13.0 - left for testing, no perf changes
2.14.2 - perf improvements (JSON pointer)
2.27.0 - huge perf improvement
2.30.2 - can have influence on perf
2.31.0 - added latest minor release for testing
cat benchmark_bundle.md benchmark_lint.md.Reference
Testing
You can see tests below in this PR.
Screenshots (optional)
Check yourself
Security
Note
Low Risk
Changes are limited to CI performance harness and reporting; no production CLI or auth/data paths are modified.
Overview
Upgrades the PR performance benchmark so CI reports bundle, lint, and check-config side by side, using median and MAD (with relative uncertainty on slower rows) instead of mean/stddev so occasional CI spikes distort results less.
The generated
hyperfinerun now uses warmup 2, writes separatebenchmark_*artifacts per command, andchart.jsbuilds a multi-column markdown table for the PR comment. Historical CLI pins in_enhancedDependenciesare trimmed to a smaller set (including 2.31.0).The performance workflow can be re-triggered via the
performance-benchmarklabel (without an empty commit), installs extra CLI versions on release PRs or when that label is present, and uses a per-run comment tag on labeled PRs so reruns stay comparable in-thread.Reviewed by Cursor Bugbot for commit 86d0e6d. Bugbot is set up for automated code reviews on this repo. Configure here.