-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add benchmarks #103
Merged
Merged
Add benchmarks #103
Changes from 8 commits
Commits
Show all changes
81 commits
Select commit
Hold shift + click to select a range
bbbd892
Start adding benchmarks
ptoffy b79b3d7
Remove approval requirement for now
ptoffy 5046e43
Update workflow
ptoffy b74536c
Update workflow
ptoffy 05422f5
Add more benchmarks
ptoffy 48bd331
Update benchmark CI with the jwt-kit one
MahdiBM 0445266
[CI] ci to get triggered on every file change at in Benchmarks folder
MahdiBM c291cdd
Try RunsOn magic cache
MahdiBM 8f15f7a
configure thresholds
MahdiBM 8efd726
remove s3-cache from this specific ci file
MahdiBM 79286b9
Create runs-on.yml
MahdiBM e35811c
Merge branch 'main' into benchmarks
MahdiBM b48c22d
update thresholds
MahdiBM 972d8af
move runs-on configuration step
MahdiBM 0125fcf
get thresholds working
MahdiBM f421467
update machine name
MahdiBM a7ce239
Update machine name
MahdiBM 214145e
use `cpuUser` instead of the other cpu metrics
MahdiBM cd0230b
update benchmarks
MahdiBM 6c5a948
Fix benchmark code
MahdiBM 9024661
rename machine
MahdiBM 86b00c7
configure RunsOn first
MahdiBM ad4c1c7
adjust benchmarks
MahdiBM 8dbe4f1
fix threshold files names
MahdiBM 663ce3b
do 1000x for cpu time
MahdiBM f998b9c
fix ranges
MahdiBM b0d7f16
revert some of the iteration changes
MahdiBM a367d13
aesthetical rename
MahdiBM 6558319
Try better CPU benchmarks
MahdiBM 900304c
use 1000 scale factor for cpu benchmarks
MahdiBM f44bbc0
try 100x with manual
MahdiBM 6ab382d
refinements
MahdiBM a1b111a
add tolerances
MahdiBM 43906e4
fix tolerance
MahdiBM 432eec0
SerializerCPUTime 10x -> 100x
MahdiBM 7ec4449
quote branch names for clarity
MahdiBM 66a211b
update thresholds
MahdiBM 4a51cfb
resolve nit picks @gwynne is going to make
MahdiBM f3ff558
increase iterations for more stability
MahdiBM 8185d07
increase max duration
MahdiBM 8f6f223
better thresholds and iterations
MahdiBM b691cde
more adjustments
MahdiBM d055d79
allocation benchmarks to only run once
MahdiBM e3197b0
increase big message size to what it should be
MahdiBM 009f2e9
decrease rounds
MahdiBM 74a0aea
minor refinements
MahdiBM 9b748cc
try something weird
MahdiBM 6128a4a
try different method
MahdiBM a89c638
Use `unsafeUninitializedCapacity` initialiser
ptoffy 79dc04d
Revert "Use `unsafeUninitializedCapacity` initialiser"
ptoffy 7528237
Try something
MahdiBM 0ec09f1
minor fixes
MahdiBM 9b30045
minor
MahdiBM 4c4cc77
refinements
MahdiBM 3259d8c
smaller chunk
MahdiBM dcb061a
use array
MahdiBM e946cb5
more fixes
MahdiBM b23f6c7
minor fixes
MahdiBM c6d84ad
refinements
MahdiBM c9ac81b
just use AsyncStream
MahdiBM 8bfa20b
better thresholds
MahdiBM 263c36c
more benchs + revert to `AsyncSyncStream` for max accuracy
MahdiBM 33efdfd
refinements
MahdiBM b71aceb
more benchmarks and refinements
MahdiBM bc3c03a
fixes
MahdiBM b5d2aa6
adjust thresholds
MahdiBM 668bb5d
minor refinement
MahdiBM 2a95328
thresholds
MahdiBM e2f37e5
thresholds
MahdiBM 3599f31
thresholds
MahdiBM 3378157
increase iterations
MahdiBM d634658
thresholds
MahdiBM 7db849a
thresholds
MahdiBM 464d97e
minor
MahdiBM 9b7ef8d
better message
MahdiBM 1651b63
thresholds
MahdiBM 6a706b9
minor
MahdiBM 82cba28
fix
MahdiBM da6eea1
better detect PR events
MahdiBM bcea652
Try thollander/actions-comment-pull-request
MahdiBM 5619d5d
minor
MahdiBM 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,265 @@ | ||
name: benchmark | ||
on: | ||
workflow_dispatch: | ||
pull_request_review: | ||
types: [submitted] | ||
pull_request: | ||
branches: [main] | ||
types: [synchronize] | ||
paths: | ||
- Sources/*.swift | ||
- Benchmarks/ | ||
- .github/workflows/benchmark.yml | ||
|
||
jobs: | ||
benchmark-vs-thresholds: | ||
# Run the job only if it's a manual workflow dispatch, or if this event is a pull-request approval event, or if someone has rerun the job. | ||
if: github.event_name == 'workflow_dispatch' || github.event.review.state == 'approved' || github.run_attempt > 1 | ||
|
||
# https://runs-on.com/features/custom-runners/ | ||
runs-on: | ||
labels: | ||
- runs-on | ||
- runner=2cpu-4ram | ||
- extras=s3-cache | ||
MahdiBM marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- run-id=${{ github.run_id }} | ||
|
||
container: swift:noble | ||
|
||
defaults: | ||
run: | ||
shell: bash | ||
|
||
env: | ||
PR_COMMENT: null # will be populated later | ||
|
||
steps: | ||
- name: Set up RunsOn magic cache | ||
uses: runs-on/action@v1 | ||
|
||
- name: Check out code | ||
uses: actions/checkout@v4 | ||
with: | ||
fetch-depth: 0 | ||
|
||
- name: Configure git | ||
run: git config --global --add safe.directory "${GITHUB_WORKSPACE}" | ||
|
||
# jemalloc is a dependency of the Benchmarking package | ||
# actions/cache will detect zstd and will become much faster. | ||
- name: Install jemalloc, curl, jq and zstd | ||
run: | | ||
set -eu | ||
|
||
apt-get update -y | ||
apt-get install -y libjemalloc-dev curl jq zstd | ||
|
||
- name: Restore .build | ||
id: restore-cache | ||
uses: actions/cache/restore@v4 | ||
with: | ||
path: Benchmarks/.build | ||
key: "swiftpm-benchmark-build-${{ runner.os }}-${{ github.event.pull_request.base.sha || github.event.after }}" | ||
restore-keys: "swiftpm-benchmark-build-${{ runner.os }}-" | ||
|
||
- name: Run benchmarks for branch ${{ github.head_ref || github.ref_name }} | ||
run: | | ||
swift package -c release --disable-sandbox \ | ||
--package-path Benchmarks \ | ||
benchmark baseline update \ | ||
'${{ github.head_ref || github.ref_name }}' | ||
|
||
- name: Read benchmark result | ||
id: read-benchmark | ||
run: | | ||
set -eu | ||
|
||
swift package -c release --disable-sandbox \ | ||
--package-path Benchmarks \ | ||
benchmark baseline read \ | ||
'${{ github.head_ref || github.ref_name }}' \ | ||
--no-progress \ | ||
--format markdown \ | ||
>> result.text | ||
|
||
# Read the result to the output of the step | ||
echo 'result<<EOF' >> $GITHUB_OUTPUT | ||
cat result.text >> $GITHUB_OUTPUT | ||
echo 'EOF' >> $GITHUB_OUTPUT | ||
|
||
- name: Compare branch ${{ github.head_ref || github.ref_name }} against thresholds | ||
id: compare-benchmark | ||
run: | | ||
set -eu | ||
|
||
TIMESTAMP=$(date -u +"%Y-%m-%d %H:%M:%S UTC") | ||
ENCODED_TIMESTAMP=$(date -u +"%Y-%m-%dT%H%%3A%M%%3A%SZ") | ||
TIMESTAMP_LINK="https://www.timeanddate.com/worldclock/fixedtime.html?iso=$ENCODED_TIMESTAMP" | ||
echo "## Benchmark check running at [$TIMESTAMP]($TIMESTAMP_LINK)" >> summary.text | ||
|
||
# Disable 'set -e' to prevent the script from exiting on non-zero exit codes | ||
set +e | ||
swift package -c release --disable-sandbox \ | ||
--package-path Benchmarks \ | ||
benchmark thresholds check \ | ||
'${{ github.head_ref || github.ref_name }}' \ | ||
--path "$PWD/Benchmarks/Thresholds/" \ | ||
--no-progress \ | ||
--format markdown \ | ||
>> summary.text | ||
echo "exit-status=$?" >> "${GITHUB_OUTPUT}" | ||
set -e | ||
|
||
echo 'summary<<EOF' >> $GITHUB_OUTPUT | ||
cat summary.text >> $GITHUB_OUTPUT | ||
echo 'EOF' >> $GITHUB_OUTPUT | ||
|
||
- name: Cache .build | ||
if: steps.restore-cache.outputs.cache-hit != 'true' | ||
uses: actions/cache/save@v4 | ||
with: | ||
path: Benchmarks/.build | ||
key: "swiftpm-benchmark-build-${{ runner.os }}-${{ github.event.pull_request.base.sha || github.event.after }}" | ||
|
||
- name: Construct comment | ||
run: | | ||
set -eu | ||
|
||
EXIT_CODE='${{ steps.compare-benchmark.outputs.exit-status }}' | ||
|
||
echo 'PR_COMMENT<<EOF' >> "${GITHUB_ENV}" | ||
|
||
# The fact that the comment starts with <!-- benchmark ci tag. exit code: ' is used | ||
# in a other steps to find this comment again. | ||
# Be wary of that when changing this line. | ||
echo "<!-- benchmark ci tag. exit code: $EXIT_CODE -->" >> "${GITHUB_ENV}" | ||
|
||
echo "" >> "${GITHUB_ENV}" | ||
|
||
echo '## [Benchmark](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) Report' >> "${GITHUB_ENV}" | ||
|
||
case "${EXIT_CODE}" in | ||
0) | ||
echo '**✅ Pull request has no significant performance differences ✅**' >> "${GITHUB_ENV}" | ||
;; | ||
1) | ||
echo '**❌ Pull request has significant performance differences 📊**' >> "${GITHUB_ENV}" | ||
;; | ||
2) | ||
echo '**❌ Pull request has significant performance regressions 📉**' >> "${GITHUB_ENV}" | ||
;; | ||
4) | ||
echo '**❌ Pull request has significant performance improvements 📈**' >> "${GITHUB_ENV}" | ||
;; | ||
*) | ||
echo '**❌ Benchmark comparison failed to complete properly with exit code $EXIT_CODE ❌**' >> "${GITHUB_ENV}" | ||
;; | ||
esac | ||
|
||
echo '<details>' >> "${GITHUB_ENV}" | ||
echo ' <summary> Click to expand comparison result </summary>' >> "${GITHUB_ENV}" | ||
echo '' >> "${GITHUB_ENV}" | ||
echo '${{ steps.compare-benchmark.outputs.summary }}' >> "${GITHUB_ENV}" | ||
echo '' >> "${GITHUB_ENV}" | ||
echo '</details>' >> "${GITHUB_ENV}" | ||
|
||
echo '' >> "${GITHUB_ENV}" | ||
|
||
echo '<details>' >> "${GITHUB_ENV}" | ||
echo ' <summary> Click to expand benchmark result </summary>' >> "${GITHUB_ENV}" | ||
echo '' >> "${GITHUB_ENV}" | ||
echo '${{ steps.read-benchmark.outputs.result }}' >> "${GITHUB_ENV}" | ||
echo '' >> "${GITHUB_ENV}" | ||
echo '</details>' >> "${GITHUB_ENV}" | ||
|
||
echo 'EOF' >> "${GITHUB_ENV}" | ||
|
||
- name: Output the comment as job summary | ||
run: echo '${{ env.PR_COMMENT }}' >> "${GITHUB_STEP_SUMMARY}" | ||
|
||
# There is a '<!-- benchmark ci tag. exit code: {some_number} -->' comment at the beginning of the benchamrk report comment. | ||
# The number in that comment is the exit code of the last benchmark run. | ||
- name: Find existing comment ID | ||
if: github.event_name == 'pull_request' | ||
id: existing-comment | ||
run: | | ||
set -eu | ||
|
||
# Known limitation: This only fetches the first 100 comments. This should not | ||
# matter much because a benchmark comment should have been sent early in the PR. | ||
curl -sL \ | ||
-X GET \ | ||
-H 'Accept: application/vnd.github+json' \ | ||
-H 'Authorization: BEARER ${{ secrets.GITHUB_TOKEN }}' \ | ||
-H 'X-GitHub-Api-Version: 2022-11-28' \ | ||
-o result.json \ | ||
'https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.number }}/comments?per_page=100' | ||
|
||
# Get the last comment that has a body that starts with '<!-- benchmark ci tag. exit code: '. | ||
# If available, we can just update this comment instead of creating new ones. | ||
EXISTING_COMMENT=$( | ||
cat result.json | | ||
jq ' | ||
[ | ||
.[] | | ||
select(.body | startswith("<!-- benchmark ci tag. exit code: ")) | ||
][-1] | ||
' | ||
) | ||
|
||
# Check if EXISTING_COMMENT is empty or null | ||
if [ "$EXISTING_COMMENT" = "null" ] || [ -z "$EXISTING_COMMENT" ]; then | ||
ID="" | ||
BODY="" | ||
PARSED_EXIT_CODE="" | ||
else | ||
ID="$(echo "$EXISTING_COMMENT" | jq '.id')" | ||
BODY="$(echo "$EXISTING_COMMENT" | jq -r '.body')" | ||
PARSED_EXIT_CODE="$(echo "$BODY" | head -n 1 | grep -o 'exit code: [0-9]\+' | grep -o '[0-9]\+')" | ||
|
||
if [ "$ID" = "null" ]; then | ||
echo "Comment ID was null? something is wrong" | ||
echo "Comment: $EXISTING_COMMENT" | ||
exit 111 | ||
fi | ||
fi | ||
|
||
# Output the results to GITHUB_OUTPUT | ||
{ | ||
echo "id=$ID" | ||
echo "exit-code=$PARSED_EXIT_CODE" | ||
echo "body<<EOF" | ||
echo "$BODY" | ||
echo "EOF" | ||
} >> "$GITHUB_OUTPUT" | ||
|
||
- name: Comment in PR | ||
if: github.event_name == 'pull_request' | ||
run: | | ||
set -eu | ||
|
||
EXISTING_COMMENT_ID="${{ steps.existing-comment.outputs.id }}" | ||
# Need to provide the argument in a way that anything in the comment itself is not evaluated. | ||
BODY_JSON="$(jq -n -c --arg COMMENT "$(echo '${{ env.PR_COMMENT }}')" '{"body": $COMMENT}')" | ||
|
||
if [ -n "$EXISTING_COMMENT_ID" ]; then | ||
echo "Will update a comment: $EXISTING_COMMENT_ID" | ||
ENDPOINT="https://api.github.com/repos/${{ github.repository }}/issues/comments/$EXISTING_COMMENT_ID" | ||
else | ||
echo "Will create a new comment" | ||
ENDPOINT="https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.number }}/comments" | ||
fi | ||
|
||
curl -sL \ | ||
-X POST \ | ||
-H "Accept: application/vnd.github+json" \ | ||
-H "Authorization: BEARER ${{ secrets.GITHUB_TOKEN }}" \ | ||
-H "X-GitHub-Api-Version: 2022-11-28" \ | ||
-d "$BODY_JSON" \ | ||
"$ENDPOINT" | ||
|
||
- name: Exit with correct status | ||
run: | | ||
EXIT_CODE='${{ steps.compare-benchmark.outputs.exit-status }}' | ||
echo "Previous exit code was: $EXIT_CODE" | ||
exit $EXIT_CODE |
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,38 @@ | ||
// swift-tools-version:6.0 | ||
|
||
import PackageDescription | ||
|
||
let package = Package( | ||
name: "Benchmarks", | ||
platforms: [ | ||
.macOS(.v13) | ||
], | ||
dependencies: [ | ||
.package(path: "../"), | ||
.package(url: "https://github.com/ordo-one/package-benchmark.git", from: "1.27.0"), | ||
], | ||
targets: [ | ||
.executableTarget( | ||
name: "Serializer", | ||
dependencies: [ | ||
.product(name: "MultipartKit", package: "multipart-kit"), | ||
.product(name: "Benchmark", package: "package-benchmark"), | ||
], | ||
path: "Serializer", | ||
plugins: [ | ||
.plugin(name: "BenchmarkPlugin", package: "package-benchmark") | ||
] | ||
), | ||
.executableTarget( | ||
name: "Parser", | ||
dependencies: [ | ||
.product(name: "MultipartKit", package: "multipart-kit"), | ||
.product(name: "Benchmark", package: "package-benchmark"), | ||
], | ||
path: "Parser", | ||
plugins: [ | ||
.plugin(name: "BenchmarkPlugin", package: "package-benchmark") | ||
] | ||
), | ||
] | ||
) |
Oops, something went wrong.
Oops, something went wrong.
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.
Not right now but I'm surprised there aren't existing actions for commenting on issues and we should look at turning this into its own action (or submitting it to the swift lang repo)
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.
Actually there are and we were using it but someone decided to do it by hand 😆 vapor/jwt-kit#222 (comment)
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 github-scripts doesn't really help, we'll end up with around the same amount of lines of code, just in js instead of bash.
Most of the code here is to keep track of the old comment and don't create new comments.
I think Tim is talking about a dedicated action that takes some parameters and just does the work. Something like
actions/checkout@v4
but for this purpose.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.
Yeah we had that before, that's what I meant with my comment in JWTKit https://github.com/thollander/actions-comment-pull-request but not sure if this can update comments too rather than creating new ones
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.
it appears it can update comments too? I'd use it if it can.
In the original CI i wrote this CI file for, i can't do that since the comment keeps some more state with it as well, but here i should be able to.
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.
Nice https://github.com/thollander/actions-comment-pull-request seems to be working well.
It apparently works the same way my curl stuff were working, but it clears 80 lines out of the CI file so better to use this instead.