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

Implementing BDN Perf pipeline #758

Open
wants to merge 48 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
a81a31e
The CI scripts, YML and config files to check perf using BDN benchmar…
darrenge Sep 9, 2024
f66f63f
Cleaned up the debug message and updated couple expected values
darrenge Sep 9, 2024
b0c8cc4
Updated SAddRem - windows - to 130 from 118. Also removed unncesssar…
darrenge Sep 9, 2024
2b0ec1f
Proof of concept for Performance run on ADO using BDN
darrenge Sep 9, 2024
ebd13e4
Comment out number of Cores check so can try proof of concept on ADO …
darrenge Sep 9, 2024
fb8ee2b
Merged
darrenge Oct 8, 2024
37d12df
Getting close to having BDN working
darrenge Oct 8, 2024
b97f807
Fixed RespLuaStress
darrenge Oct 8, 2024
ba45091
Fixed issue in Lua Runner Stress
darrenge Oct 8, 2024
bff5a21
Updated config values, set charting and commit comment as separate tasks
darrenge Oct 9, 2024
83169ef
Updated config values
darrenge Oct 9, 2024
cd17683
Minor change to YML (remove pull requests) and updated LuaRunner conf…
darrenge Oct 9, 2024
b334f65
One more config value fix
darrenge Oct 9, 2024
4df29ee
Added a task to do the push
darrenge Oct 9, 2024
82f6f15
Putting back to push chart in the chart task and not a separate one
darrenge Oct 10, 2024
58038d1
Moved the permissions to full workflow
darrenge Oct 10, 2024
5875d87
Just minor to add commits
darrenge Oct 10, 2024
d69a07b
Adding actions: write permit since actions default to read only
darrenge Oct 10, 2024
68d0972
Set up the commit comment for BDN as well as summary to show current …
darrenge Oct 14, 2024
f8ddb6c
A few fixes
darrenge Oct 14, 2024
8e05865
Updated BDN perf YML, config files and ps1 script
darrenge Oct 16, 2024
4008726
Versioning issue with BDN test files, fixed now
darrenge Oct 16, 2024
82881ff
Merge branch 'main' into darrenge/BDNPerf
darrenge Oct 16, 2024
cdf2d25
Bump threshold to 35% (135%) to trigger perf failure
darrenge Oct 16, 2024
00b3674
Set to Linux only to reduce number of consectutive tests which caused…
darrenge Oct 16, 2024
414168e
Just Windows only test run
darrenge Oct 16, 2024
28ccbb4
Putting it back to both OS for the tests
darrenge Oct 16, 2024
23296d6
Set threshhold to 50% (150%) as seeing BDN ran twice on same code som…
darrenge Oct 16, 2024
1f36768
Big change where using continuousbenchmark branch for all data for co…
darrenge Oct 18, 2024
ef0cf7b
Updated ClusterBench to get main changes
darrenge Oct 18, 2024
df16e9c
Merge branch 'main' into darrenge/BDNPerf
darrenge Oct 18, 2024
d5953a2
Fix issue with Cluster BDN is not able to access folder so setting to…
darrenge Oct 18, 2024
d58bf0d
Bumping threshold to 50% (150%) as mean value in RespParseStress can …
darrenge Oct 18, 2024
6414fac
Merged Main
darrenge Oct 21, 2024
75e78c3
First update
darrenge Oct 22, 2024
40e111c
Merge branch 'main' into darrenge/BDNPerf
darrenge Oct 22, 2024
e22218c
Hiding the Gen0 column for Lua benchmark
darrenge Oct 22, 2024
b609c0c
Merge branch 'main' into darrenge/BDNPerf
darrenge Oct 28, 2024
f9a963b
Updated BDN test infrastructure to match the new BDN org.
darrenge Oct 28, 2024
665bda1
Fix bug in ObjectOperations
darrenge Oct 28, 2024
0d1fd6f
Set max items on chart to 50 ... can modify later if want
darrenge Oct 28, 2024
da030c6
Merge branch 'main' into darrenge/BDNPerf
darrenge Oct 29, 2024
6725d99
Added the run on push to main
darrenge Oct 29, 2024
da6b517
Put Performance Pipeline back
darrenge Oct 29, 2024
3871a3f
Fixed the default case when no parameters sent to ps1
darrenge Oct 29, 2024
3d6d7b2
Merge branch 'main' into darrenge/BDNPerf
darrenge Oct 30, 2024
591fadc
Merge branch 'main' into darrenge/BDNPerf
darrenge Oct 31, 2024
bf06727
Cleaned up config files:
darrenge Oct 31, 2024
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
88 changes: 83 additions & 5 deletions .github/workflows/ci-bdnbenchmark.yml
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
name: Garnet CI BDN.benchmark
on:
workflow_dispatch:

push:
branches:
- main

env:
DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1
DOTNET_NOLOGO: true

permissions:
actions: write
deployments: write #permission to deploy GitHub pages website
contents: write # permission to update benchmark contents in gh-pages branch

jobs:
changes:
name: Check for changes
runs-on: ubuntu-latest # don't need matrix to test where the changes were made in code
permissions:
pull-requests: read
contents: read
runs-on: ubuntu-latest
steps:
- name: Check out code
uses: actions/checkout@v4
Expand All @@ -25,3 +30,76 @@ jobs:
- 'website/**'
bdnbenchmark:
- '!((*.md)|(website/**))'

# Job to build and run BDN.benchmark
build-run-bdnbenchmark:
name: BDNBenchmark
needs: [changes]
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest, windows-latest ]
framework: [ 'net8.0' ]
configuration: [ 'Release' ]
test: [ 'Operations.BasicOperations', 'Operations.ObjectOperations', 'Cluster.ClusterMigrate', 'Cluster.ClusterOperations', 'Lua.LuaScripts' ]
steps:
- name: Check out code
uses: actions/checkout@v4
- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: 8.0.x
- name: Install dependencies
run: dotnet restore

- name: Run BDN.benchmark Perf Test
run: ./test/BDNPerfTests/run_bdnperftest.ps1 ${{ matrix.test }}
shell: pwsh
continue-on-error: false

- name: Upload test results to artifacts
uses: actions/upload-artifact@v4
with:
name: Results-${{ matrix.os }}-${{ matrix.framework }}-${{ matrix.configuration }}-${{ matrix.test }}
path: |
./test/BDNPerfTests/results
if: ${{ always() }}

- name: Upload Error Log to artifacts
uses: actions/upload-artifact@v4
with:
name: ErrorLog-${{ matrix.os }}-${{ matrix.framework }}-${{ matrix.configuration }}-${{ matrix.test }}
path: |
./test/BDNPerfTests/errorlog
if: ${{ always() }}

# Run `github-action-benchmark` action to get Commit Comment
- name: Store benchmark result for commit comment
uses: benchmark-action/github-action-benchmark@v1
with:
tool: 'benchmarkdotnet'
output-file-path: ./test/BDNPerfTests/BenchmarkDotNet.Artifacts/results/BDN.benchmark.${{ matrix.test }}-report-full-compressed.json
github-token: ${{ secrets.GITHUB_TOKEN }}
alert-threshold: '150%'
comment-on-alert: true
fail-on-alert: true
alert-comment-cc-users: '@darrenge'
summary-always: false
gh-pages-branch: 'continuousbenchmark'
benchmark-data-dir-path: 'website/static/charts'
auto-push: true

# Run `github-action-benchmark` action for the Continuous Benchmarking Charts
- name: Store benchmark result for charts
uses: benchmark-action/github-action-benchmark@v1
with:
name: ${{matrix.test}} (${{matrix.os}} ${{matrix.framework}} ${{matrix.configuration}})
tool: 'benchmarkdotnet'
output-file-path: ./test/BDNPerfTests/BenchmarkDotNet.Artifacts/results/BDN.benchmark.${{ matrix.test }}-report-full-compressed.json
github-token: ${{ secrets.GITHUB_TOKEN }}
summary-always: true
gh-pages-branch: 'continuousbenchmark'
benchmark-data-dir-path: 'website/static/charts'
auto-push: true
max-items-in-chart: 50
2 changes: 2 additions & 0 deletions benchmark/BDN.benchmark/Lua/LuaScripts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Columns;
using Garnet.server;

namespace BDN.benchmark.Lua
Expand All @@ -10,6 +11,7 @@ namespace BDN.benchmark.Lua
/// Benchmark for Lua
/// </summary>
[MemoryDiagnoser]
[HideColumns(Column.Gen0)]
public unsafe class LuaScripts
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"configuration": "Release",
"framework": "net8.0",
"filter": "BDN.benchmark.Cluster.ClusterMigrate.*",

"expectedGet_None_AllocatedValue_linux": 0,
"expectedSet_None_AllocatedValue_linux": 0,
"expectedMGet_None_AllocatedValue_linux": 0,
"expectedMSet_None_AllocatedValue_linux": 0,

"expectedGet_None_AllocatedValue_win": 0,
"expectedSet_None_AllocatedValue_win": 0,
"expectedMGet_None_AllocatedValue_win": 0,
"expectedMSet_None_AllocatedValue_win": 0,

"acceptableAllocatedRange": 10,
"acceptableError": ".1",
"acceptableStdDev": ".1"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"configuration": "Release",
"framework": "net8.0",
"filter": "BDN.benchmark.Cluster.ClusterOperations.*",

"expectedGet_DSV_AllocatedValue_linux": 0,
Copy link
Contributor

@badrishc badrishc Oct 31, 2024

Choose a reason for hiding this comment

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

i think we should only have to add files and entries with a non-zero allocated target. the script should assume, if it does not find a benchmark entry, that the allocated should be zero. that way this entire file gets deleted. and, many lines in other files get deleted as well. will make maintenance a tad easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see where you are coming from but not sure I agree with this. Maintenance is easiest if it is consistent so we know one config file per BDN run. Also, assuming it should be zero if not there is an assumption and it might be better to specifically state to show we know it should be zero. Currently, the BDN name/ filter is in the config file as well, so that would be a change to only just use the test name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are only pulling values we need for the specific test (based on other comment), we need to know what values we need for that specific test. Those specific values are in the config file, so I think it only makes sense to have all config files even if the values are 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

we could have one config file that has a section per test. for every test we add we would simply run the test, and for every result, we would comare it to either zero (by default) or an override (if specified in the JSON).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I guess that would be another way to implement it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to implement it with one config file for all BDN test runs

"expectedSet_DSV_AllocatedValue_linux": 0,
"expectedMGet_DSV_AllocatedValue_linux": 0,
"expectedMSet_DSV_AllocatedValue_linux": 0,
"expectedCPBSet_DSV_AllocatedValue_linux": 0,
"expectedGet_None_AllocatedValue_linux": 0,
"expectedSet_None_AllocatedValue_linux": 0,
"expectedMGet_None_AllocatedValue_linux": 0,
"expectedMSet_None_AllocatedValue_linux": 0,
"expectedCPBSet_None_AllocatedValue_linux": 0,

"expectedGet_DSV_AllocatedValue_win": 0,
"expectedSet_DSV_AllocatedValue_win": 0,
"expectedMGet_DSV_AllocatedValue_win": 0,
"expectedMSet_DSV_AllocatedValue_win": 0,
"expectedCPBSet_DSV_AllocatedValue_win": 0,
"expectedGet_None_AllocatedValue_win": 0,
"expectedSet_None_AllocatedValue_win": 0,
"expectedMGet_None_AllocatedValue_win": 0,
"expectedMSet_None_AllocatedValue_win": 0,
"expectedCPBSet_None_AllocatedValue_win": 0,

"acceptableAllocatedRange": 10,
darrenge marked this conversation as resolved.
Show resolved Hide resolved
"acceptableError": ".1",
"acceptableStdDev": ".1"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"configuration": "Release",
darrenge marked this conversation as resolved.
Show resolved Hide resolved
"framework": "net8.0",
"filter": "BDN.benchmark.Lua.*",

"expectedScript1_None_AllocatedValue_linux": 24,
darrenge marked this conversation as resolved.
Show resolved Hide resolved
"expectedScript2_None_AllocatedValue_linux": 144,
"expectedScript3_None_AllocatedValue_linux": 240,
"expectedScript4_None_AllocatedValue_linux": 776,

"expectedScript1_None_AllocatedValue_win": 24,
"expectedScript2_None_AllocatedValue_win": 144,
"expectedScript3_None_AllocatedValue_win": 240,
"expectedScript4_None_AllocatedValue_win": 776,

"acceptableAllocatedRange": 10,
"acceptableError": ".1",
"acceptableStdDev": ".1"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"configuration": "Release",
darrenge marked this conversation as resolved.
Show resolved Hide resolved
"framework": "net8.0",
"filter": "BDN.benchmark.Operations.BasicOperations.*",

"expectedInlinePing_ACL_AllocatedValue_linux": 0,
"expectedInlinePing_AOF_AllocatedValue_linux": 0,
"expectedInlinePing_None_AllocatedValue_linux": 0,

"expectedInlinePing_ACL_AllocatedValue_win": 0,
"expectedInlinePing_AOF_AllocatedValue_win": 0,
"expectedInlinePing_None_AllocatedValue_win": 0,

"acceptableAllocatedRange": 10,
"acceptableError": ".1",
"acceptableStdDev": ".1"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{
"configuration": "Release",
"framework": "net8.0",
"filter": "BDN.benchmark.Operations.ObjectOperations.*",

"expectedZAddRem_ACL_AllocatedValue_linux": 18400,
darrenge marked this conversation as resolved.
Show resolved Hide resolved
"expectedLPushPop_ACL_AllocatedValue_linux": 14400,
"expectedSAddRem_ACL_AllocatedValue_linux": 12800,
"expectedHSetDel_ACL_AllocatedValue_linux": 43201,
"expectedMyDictSetGet_ACL_AllocatedValue_linux": 24000,
"expectedCustomProcSet_ACL_AllocatedValue_linux": 0,

"expectedZAddRem_AOF_AllocatedValue_linux": 18400,
"expectedLPushPop_AOF_AllocatedValue_linux": 14400,
"expectedSAddRem_AOF_AllocatedValue_linux": 12800,
"expectedHSetDel_AOF_AllocatedValue_linux": 43201,
"expectedMyDictSetGet_AOF_AllocatedValue_linux": 24000,
"expectedCustomProcSet_AOF_AllocatedValue_linux": 0,

"expectedZAddRem_None_AllocatedValue_linux": 18400,
"expectedLPushPop_None_AllocatedValue_linux": 14400,
"expectedSAddRem_None_AllocatedValue_linux": 12800,
"expectedHSetDel_None_AllocatedValue_linux": 43201,
"expectedMyDictSetGet_None_AllocatedValue_linux": 24000,
"expectedCustomProcSet_None_AllocatedValue_linux": 0,


"expectedZAddRem_ACL_AllocatedValue_win": 18400,
"expectedLPushPop_ACL_AllocatedValue_win": 14400,
"expectedSAddRem_ACL_AllocatedValue_win": 12800,
"expectedHSetDel_ACL_AllocatedValue_win": 43201,
"expectedMyDictSetGet_ACL_AllocatedValue_win": 24000,
"expectedCustomProcSet_ACL_AllocatedValue_win": 0,

"expectedZAddRem_AOF_AllocatedValue_win": 18400,
"expectedLPushPop_AOF_AllocatedValue_win": 14400,
"expectedSAddRem_AOF_AllocatedValue_win": 12800,
"expectedHSetDel_AOF_AllocatedValue_win": 43201,
"expectedMyDictSetGet_AOF_AllocatedValue_win": 24000,
"expectedCustomProcSet_AOF_AllocatedValue_win": 0,

"expectedZAddRem_None_AllocatedValue_win": 18400,
"expectedLPushPop_None_AllocatedValue_win": 14400,
"expectedSAddRem_None_AllocatedValue_win": 12800,
"expectedHSetDel_None_AllocatedValue_win": 43201,
"expectedMyDictSetGet_None_AllocatedValue_win": 24000,
"expectedCustomProcSet_None_AllocatedValue_win": 0,

"acceptableAllocatedRange": 10,
"acceptableError": ".1",
"acceptableStdDev": ".1"
}

Loading