new(ci): added an initial perf-related CI. #1918
Conversation
Signed-off-by: Federico Di Pierro <[email protected]>
poiana
added
kind/feature
|
/milestone 0.18.0 |
|
|
||
| jobs: | ||
| perf-libs-linux-amd64: | ||
| runs-on: [ "self-hosted", "linux", "X64" ] |
There was a problem hiding this comment.
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
| cd build | ||
| sudo perf record --call-graph dwarf -o perf.data -q libsinsp/test/unit-test-libsinsp | ||
|
|
||
| - name: Archive master perf report |
There was a problem hiding this comment.
On master, archive the perf data.
| path: build/perf.data | ||
| if-no-files-found: error | ||
|
|
||
| - name: Download latest master report |
There was a problem hiding this comment.
On PR, download latest master data and diff our just built perf.data with the master one.
|
|
||
| # 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 |
There was a problem hiding this comment.
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.
| exit 1 | ||
| fi | ||
|
|
||
| - name: Save PR info |
There was a problem hiding this comment.
Last 2 steps are needed to be able to trigger create-comment workflow with all needed data.
| jobs: | ||
| upload: | ||
| runs-on: ubuntu-latest | ||
| if: github.event.workflow_run.event == 'pull_request' |
There was a problem hiding this comment.
Of course this needs to run only when triggered from a PR.
There was a problem hiding this comment.
This file was copied from https://github.com/falcosecurity/rules/blob/main/.github/workflows/create-comment.yaml
| fi | ||
|
|
||
| - name: Save PR info | ||
| if: always() && github.event_name == 'pull_request' |
There was a problem hiding this comment.
Even if compare step above did fail, ie exit 1 was called because we are above threshold.
|
@LucaGuerra could you review this? Thanks! |
| 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 |
There was a problem hiding this comment.
probably 30 days is a little bit too much, usually, we merge at least a PR every day 😆
There was a problem hiding this comment.
Yep but i was thinking about august :D
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, FedeDP, leogr The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area CI
Does this PR require a change in the driver versions?
What this PR does / why we need it:
Split from #1910. Added a CI around
perftool.For now, we only run libsinsp unit tests with perf. In the future we might want to add:
valgrind massiftoolAlso, the new
perf-libs-linux-amd64check needs to be added to required status checks once we find a good enough threshold.The CI will fail because it expects master CI to upload perf reports.
I will rebase #1910 once this is merged and expect the PR comment to be created.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: