-
Notifications
You must be signed in to change notification settings - Fork 280
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
[ci] add gitlab pipeline for criterion benchmarks #422
Merged
Merged
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
eccb2ea
add gitlab pipeline
sergejparity 410c90c
change job name
sergejparity d2c7401
add fetch
sergejparity 55cd0af
update submodules
sergejparity 9c39f52
cleanup
sergejparity ddcb3fa
Merge branch 'master' into sk-add-gitlab-pipeline
sergejparity 0341a77
add report generation and post
sergejparity 27c855d
Merge branch 'master' into sk-add-gitlab-pipeline
sergejparity 8ba2dfe
benchmark report update
sergejparity ed08dfd
Merge branch 'master' into sk-add-gitlab-pipeline
sergejparity 9b119d6
add debug output
sergejparity 9f26a68
more debugging
sergejparity 89f75ec
more debugging
sergejparity d0d9037
more debugging
sergejparity 07e3001
fix sed expr
sergejparity 2104958
fix sed expr
sergejparity 880312d
fix sed expr
sergejparity 22b5ad2
fix sed expr
sergejparity 0127464
fix sed expr
sergejparity 66d3699
fix sed expr
sergejparity 5fb1be0
fix sed expr
sergejparity e00c1d2
fix sed expr
sergejparity 173cadb
cleanup script
sergejparity ae857eb
fix quotes
sergejparity e4367b9
add debug output
sergejparity 2b616e2
add debug output
sergejparity aafb623
remove debug
sergejparity 8e88798
update message template
sergejparity a318e4c
update message template
sergejparity aad4cae
fix message template
sergejparity c8fa34d
reformat code
sergejparity 0582356
reformat code
sergejparity bdfa811
decorate fonts
sergejparity e916aec
remove debug output
sergejparity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# paritytech/wasmi | ||
|
||
stages: | ||
- benchmark | ||
|
||
default: | ||
retry: | ||
max: 2 | ||
when: | ||
- runner_system_failure | ||
- unknown_failure | ||
- api_failure | ||
|
||
.kubernetes-env: &kubernetes-env | ||
image: "paritytech/ci-linux:production" | ||
tags: | ||
- kubernetes-parity-build | ||
|
||
.docker-env: &docker-env | ||
image: "paritytech/ci-linux:production" | ||
interruptible: true | ||
tags: | ||
- linux-docker | ||
|
||
# benchmark | ||
criterion-benchmark: | ||
stage: benchmark | ||
rules: | ||
- if: $CI_COMMIT_REF_NAME =~ /^[0-9]+$/ # PRs | ||
<<: *docker-env | ||
script: | ||
- git fetch | ||
- git submodule update --init --recursive | ||
- git checkout master | ||
# on master | ||
- cargo bench --bench benches -- --noplot --save-baseline master | tee bench-report-master.txt | ||
# on PR | ||
- git checkout $CI_COMMIT_SHA | ||
- cargo bench --bench benches -- --noplot --baseline master | tee bench-report-pr.txt | ||
- bash ./scripts/ci/benchmarks-report.sh bench-report-master.txt bench-report-pr.txt |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
#!/bin/bash | ||
|
||
# Takes raw reports from | ||
# "cargo bench --bench benches -- --noplot --save-baseline master" output as 1st argument | ||
# "cargo bench --bench benches -- --noplot --baseline master" output as 2nd argument | ||
# Parses them to json and posts formatted results to PR on a GitHub as a comment | ||
set -eu | ||
set -o pipefail | ||
|
||
PR_COMMENTS_URL="https://api.github.com/repos/paritytech/wasmi/issues/${CI_COMMIT_BRANCH}/comments" | ||
|
||
# master report to json | ||
echo "PARSING MASTER REPORT" | ||
sed -e 's/^Found.*//g' \ | ||
-e 's/^\s\+[[:digit:]].*$//g' \ | ||
-e 's/\//_/g' \ | ||
-e 's/^[a-z0-9_]\+/"&": {/g' \ | ||
-e 's/time:\s\+\[.\{10\}/"time": "/g' \ | ||
-e 's/.\{10\}\]/"},/g' \ | ||
-e '1s/^/{\n/g' \ | ||
-e '/^$/d' \ | ||
-e 's/ */ /g' \ | ||
-e 's/^ *\(.*\) *$/\1/' $1 \ | ||
| sed -z 's/.$//' \ | ||
| sed -e '$s/.$/}/g' \ | ||
| tee target/criterion/output_master.json | ||
|
||
# PR report to json | ||
sed -e 's/^Found.*//g' \ | ||
-e 's/^\s\+[[:digit:]].*//g' \ | ||
-e 's/\//_/g' \ | ||
-e 's/^[a-z0-9_]\+/"&": {/g' \ | ||
-e 's/time:\s\+\[.\{10\}/"time": "/g' \ | ||
-e 's/.\{10\}\]$/",/g' \ | ||
-e 's/change:\s.\{10\}/"change":"/g' \ | ||
-e 's/\s[-+].*$/",/g' \ | ||
-e 's/\(No\|Ch\).*$/"perf_change":":white_circle:"},/' \ | ||
-e 's/Performance has regressed./"perf_change":":red_circle:"},/' \ | ||
-e 's/Performance has improved./"perf_change":":green_circle:"},/' \ | ||
-e '1s/^/{\n/g' \ | ||
-e '/^$/d' \ | ||
-e 's/ */ /g' \ | ||
-e 's/^ *\(.*\) *$/\1/' $2 \ | ||
| sed -z 's/.$//' \ | ||
| sed -e '$s/.$/}/g' \ | ||
| tee target/criterion/output_pr.json | ||
|
||
cd target/criterion | ||
|
||
# Prepare report table | ||
for d in */; do | ||
d=${d::-1} | ||
echo -n "| \`${d}\` "\ | ||
"| $(cat output_master.json | jq .${d}.time | tr -d '"') "\ | ||
"| $(cat output_pr.json | jq .${d}.time | tr -d '"') "\ | ||
"| $(cat output_pr.json | jq .${d}.perf_change | tr -d '"') "\ | ||
"$(cat output_pr.json | jq .${d}.change | tr -d '"') |\n" >> bench-final-report.txt | ||
done | ||
|
||
RESULT=$(cat bench-final-report.txt) | ||
|
||
# Check whether comment from paritytech-cicd-pr already exists | ||
EXISTING_COMMENT_URL=$(curl --silent $PR_COMMENTS_URL \ | ||
| jq -r ".[] \ | ||
| select(.user.login == \"paritytech-cicd-pr\") \ | ||
| .url" \ | ||
| head -n1) | ||
|
||
# If there is already a comment by the user `paritytech-cicd-pr` in the PR which triggered | ||
# this run, then we can just edit this comment (using `PATCH` instead of `POST`). | ||
REQUEST_TYPE="POST" | ||
if [ ! -z "$EXISTING_COMMENT_URL" ]; then | ||
REQUEST_TYPE="PATCH"; | ||
PR_COMMENTS_URL="$EXISTING_COMMENT_URL" | ||
fi | ||
|
||
echo "Comment will be posted here $PR_COMMENTS_URL" | ||
|
||
# POST/PATCH comment to the PR | ||
curl -X ${REQUEST_TYPE} ${PR_COMMENTS_URL} -v \ | ||
-H "Cookie: logged_in=no" \ | ||
-H "Authorization: token ${GITHUB_PR_TOKEN}" \ | ||
-H "Content-Type: application/json; charset=utf-8" \ | ||
-d $"{ \ | ||
\"body\": \ | ||
\"## CRITERION BENCHMARKS ## \n\n \ | ||
|BENCHMARK|MASTER|PR|Diff|\n \ | ||
|---|---|---|---|\n \ | ||
${RESULT}\n\n \ | ||
[Link to pipeline](${CI_JOB_URL}) \" \ | ||
}" |
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.
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.
As a follow up to this PR we should try to make this part more readable.
For example criterion supports JSON output as well: https://bheisler.github.io/criterion.rs/book/cargo_criterion/external_tools.html
And with tools such as
jq
we could significantly make extracting information more readable.I will merge this PR and propose a follow-up PR for you. :) Is that okay with you?
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.
Agree. My homegrown parsing "engine" looks ugly. And definitely can be improved.
But json export tool you mention is a part of
cargo-criterion
which is under development. By the way, have you tried it in action?Reports from
cargo bench
also produces jsons with raw data intarget/criterion
. I've tried to use them as well, but found that even it is possible get execution times and other metrics. Only thing stopped me from using it is that I forced to write my own statistics analysis tool to interpret them :)So parsing raw command line output to json appeared easier solution.
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.
Upd: tried to install and use
cargo-criterion
and first attempt was not successful:Double checked
benches/wasm/wasm_kernel/target/wasm32-unknown-unknown/release/wasm_kernel.wasm
is in place. What could be the reason for failure.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.
The problem with raw command line output compared to the JSON output (or genreally machine readable output) is that the former is subject to change whereas the latter is kinda guaranteed to be stable to not break dependents. Therefore as a long term non-brittle solution we really really want to read output that is intended to be machine readable.
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.
You are probably doing this in the root directory instead of the
wasmi_v1
subdirectory.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.
You are right. Now it works, at first glance json's produced by
cargo-criterion
are almost what is needed. Of course some processing also is needed to produce readable output, like time conversion from ns into more convenient units. But at least we'll have an out-of-the-box machine readable data