-
Notifications
You must be signed in to change notification settings - Fork 325
Format benchmarks #10550
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
base: master
Are you sure you want to change the base?
Format benchmarks #10550
Conversation
a25c362 to
5b5cc66
Compare
.gitlab/scripts/benchmark-compare.sh
Outdated
| if [[ "${BENCHMARK_TYPE}" == "startup" ]]; then | ||
| "${SCRIPT_DIR}/append-startup-report" "${TYPE_DIR}" >> "${TYPE_DIR}/comparison-baseline-vs-candidate.md" || true | ||
| elif [[ "${BENCHMARK_TYPE}" == "load" ]]; then | ||
| "${SCRIPT_DIR}/append-load-report" "${TYPE_DIR}" >> "${TYPE_DIR}/comparison-baseline-vs-candidate.md" || true | ||
| elif [[ "${BENCHMARK_TYPE}" == "dacapo" ]]; then | ||
| "${SCRIPT_DIR}/append-dacapo-report" "${TYPE_DIR}" >> "${TYPE_DIR}/comparison-baseline-vs-candidate.md" || true | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should not need to append reports. each comparison-baseline-vs-candidate.md is just in its respective benchmark directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To follow up on our discussion from yesterday
The steps I see to make it cleaner are the following:
- Convert the results using bp-analyzer since now we have the conversion scripts in place
- Merge the runs that need to be merged (like the 10 runs from decapo into one) like done in here https://github.com/DataDog/dd-trace-java/pull/10550/changes#:~:text=benchmark_analyzer%20merge%20%5C,%24%7Bk6_files%5B%40%5D%7D%22
- compare the results to output in md format using bp_analyzer
example:
benchmark_analyzer compare pairwise \
--baseline='{"baseline_or_candidate":"baseline"}' \
--candidate='{"baseline_or_candidate":"candidate"}' \
--format='$BP_FORMAT' \
--outpath="$ARTIFACTS_DIR/comparison-baseline-vs-candidate.md" \
"$ARTIFACTS_DIR"/baseline*.converted.json "$ARTIFACTS_DIR"/candidate*.converted.json
Adapt the blob pattern as needed
- you can skip the setup for pr commenting using the bp-tools image and piping the md output to the pr-commenter like
cat "$ARTIFACTS_DIR/comparison-baseline-vs-candidate.md" | pr-commenter --for-repo="$CI_PROJECT_NAME" --for-pr="$CI_COMMIT_REF_NAME" --header='$BP_HEADER' --on-duplicate=replace
(if you want to append multiple files as comment you can use --on-duplicate=minimize)
Let me know if anything is still unclear 🙏
What Does This Do
WIP
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.