Skip to content
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

new(ci): added an initial perf-related CI. #1918

Merged
merged 1 commit into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions .github/workflows/create-comment.yml
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# NOTE: This has read-write repo token and access to secrets, so this must
# not run any untrusted code.
# see: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
name: Comment on the pull request

on:
workflow_run:
workflows: ["Perf CI"]
types:
- completed

jobs:
upload:
runs-on: ubuntu-latest
if: github.event.workflow_run.event == 'pull_request'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course this needs to run only when triggered from a PR.

steps:
- name: 'Download artifact'
uses: actions/[email protected]
with:
script: |
var artifacts = await github.rest.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: ${{github.event.workflow_run.id }},
});
var matchArtifact = artifacts.data.artifacts.filter((artifact) => {
return artifact.name == "pr"
})[0];
var download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
var fs = require('fs');
fs.writeFileSync('${{github.workspace}}/pr.zip', Buffer.from(download.data));

- name: 'Unpack artifact'
run: unzip pr.zip

- name: 'Comment on PR'
uses: actions/[email protected]
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
var fs = require('fs');
var issue_number = Number(fs.readFileSync('./NR'));
var comment_body = fs.readFileSync('./COMMENT');
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: issue_number,
body: comment_body.toString('utf8')
});
104 changes: 104 additions & 0 deletions .github/workflows/perf.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
name: Perf CI
on:
pull_request:
push:
branches:
- master
workflow_dispatch:

# Checks if any concurrent jobs under the same pull request or branch are being executed
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}-perf
cancel-in-progress: true

jobs:
perf-libs-linux-amd64:
runs-on: [ "self-hosted", "linux", "X64" ]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use our own cncf node because:

  • we need full hw access for perf counters
  • we need hw to stay stable to be able to properly diff perf datas

steps:
- name: Checkout Libs ⤵️
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
fetch-depth: 0

- name: Install deps ⛓️
run: |
sudo apt update && sudo apt install -y --no-install-recommends ca-certificates cmake build-essential git clang llvm pkg-config autoconf automake libtool libelf-dev wget libc-ares-dev libcurl4-openssl-dev libssl-dev libtbb-dev libjq-dev libjsoncpp-dev libgrpc++-dev protobuf-compiler-grpc libgtest-dev libprotobuf-dev
sudo .github/install-deps.sh

- name: Build
run: |
mkdir -p build
cd build && cmake -DUSE_BUNDLED_DEPS=False ../
make unit-test-libsinsp -j4

- name: Run Perf
shell: bash
run: |
cd build
sudo perf record --call-graph dwarf -o perf.data -q libsinsp/test/unit-test-libsinsp

- name: Archive master perf report
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On master, archive the perf data.

if: github.event_name == 'push'
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
with:
name: perf_report
retention-days: 30 # 30 days because this is the artifact on master; we need to retain it to be able to properly diff it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably 30 days is a little bit too much, usually, we merge at least a PR every day 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep but i was thinking about august :D

path: build/perf.data
if-no-files-found: error

- name: Download latest master report
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On PR, download latest master data and diff our just built perf.data with the master one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

if: github.event_name == 'pull_request'
uses: dawidd6/action-download-artifact@v6
with:
workflow: ci.yml
branch: master
event: push
name: perf_report

- name: Diff from master
if: github.event_name == 'pull_request'
run: |
sudo perf diff perf.data build/perf.data -d unit-test-libsinsp -b -o 0 --percentage relative -q &> perf_diff.txt

- name: Archive perf diff
if: github.event_name == 'pull_request'
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
with:
name: perf_diff
path: perf_diff.txt
if-no-files-found: error

# Check will fail if there is any function slowed down >2%
# But we will always comment with the perf diff from master
- name: Check > 2% threshold
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Threshold is set to >2%; we can later refine it.
A PR comment with full perf diff will always be added; useful for now for testing, but i think it is worth keeping it.

if: github.event_name == 'pull_request'
id: compare
run: |
echo "comment_file=perf_diff.txt" >> $GITHUB_OUTPUT
awk '{if (substr($2,RSTART+RLENGTH)+0 > 2) print }' perf_diff.txt &> perf_diff_above_thresh.txt
if [ -s perf_diff_above_thresh.txt ]; then
exit 1
fi

- name: Save PR info
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last 2 steps are needed to be able to trigger create-comment workflow with all needed data.

if: always() && github.event_name == 'pull_request'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if compare step above did fail, ie exit 1 was called because we are above threshold.

run: |
mkdir -p ./pr
echo ${{ github.event.number }} > ./pr/NR
touch ./pr/COMMENT
echo "# Perf diff from master" >> ./pr/COMMENT
echo "\`\`\`" >> ./pr/COMMENT
cat "${{ steps.compare.outputs.comment_file }}" >> ./pr/COMMENT
echo "\`\`\`" >> ./pr/COMMENT
echo Uploading PR info...
cat ./pr/COMMENT
echo ""

- name: Upload PR info as artifact
if: always() && github.event_name == 'pull_request'
uses: actions/upload-artifact@v4
with:
name: pr
path: pr/
retention-days: 1
if-no-files-found: warn
Loading