Skip to content
Closed
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
16 changes: 16 additions & 0 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ Triggered on every commit on `main`, twice daily on week days, and manually, but

Tests are run on two different testbeds for comparison: gha-vmss-d16av6-ci (d16av6 VMs) and gha-c-aci-ci (C-ACI with 16 cores and 32Gb RAM).

Both jobs use the `bencher` configure and build presets in [`CMakePresets.json`](../../CMakePresets.json). With the build dependencies installed, run these commands from the repository root to use the same build settings locally:

```bash
cmake --preset bencher
cmake --build --preset bencher
cd build
./tests.sh -VV -L benchmark
./tests.sh -VV -L perf -C perf
```

On SNP, add `-E task_bench` to the microbenchmark command to match the workflow.

The configure preset selects Ninja, `RelWithDebInfo`, and two worker threads, using the existing `build/` directory. Additional configure options can still be supplied, for example `cmake --preset bencher -DWORKER_THREADS=4`. Local presets can be defined in the ignored `CMakeUserPresets.json` file.

The build preset selects the `ccf_bencher` aggregate target rather than building every target. `add_picobench()` automatically registers its executable with this target. Each perf `add_e2e_test()` must declare its dependencies with `BUILD_TARGETS`, using CMake target names such as `logging`, not executable paths such as `samples/apps/logging/logging`. Declare these dependencies alongside each perf test so the build follows the registered tests without parsing CTest output. Normal builds without presets remain unchanged.

File: `bencher.yml`
3rd party dependencies: None

Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/bencher.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ jobs:

- name: Build and run virtual perf tests
run: |
set -eo pipefail
git config --global --add safe.directory /__w/CCF/CCF
mkdir build
cmake --preset bencher
cmake --build --preset bencher

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason to switch to cmake --build instead of (cd build &&) ninja?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Surely you'd want a single cmake --build command, that's the point of presets no?

cd build
cmake -GNinja -DWORKER_THREADS=2 ..
ninja
# Microbenchmarks
./tests.sh -VV -L benchmark
# End to end performance tests
Expand Down Expand Up @@ -178,11 +178,11 @@ jobs:

- name: Build and run SNP perf tests
run: |
set -eo pipefail
git config --global --add safe.directory /__w/CCF/CCF
mkdir build
cmake --preset bencher
cmake --build --preset bencher
cd build
cmake -GNinja -DWORKER_THREADS=2 ..
ninja
# Microbenchmarks
./tests.sh -VV -L benchmark -E task_bench
# End to end performance tests
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ build*/
Debug*/
Release*/
CMakeFiles/
/CMakeUserPresets.json
.vscode/
.vs/
libuv/
Expand Down
8 changes: 8 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,8 @@ install(TARGETS verify_attestation DESTINATION bin)
if(BUILD_TESTS)
enable_testing()

add_custom_target(ccf_bencher)

# Unit tests
if(BUILD_UNIT_TESTS)
add_test(
Expand Down Expand Up @@ -1208,6 +1210,7 @@ if(BUILD_TESTS)
PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/commit_latency.py
LABEL perf
CONFIGURATIONS perf
BUILD_TARGETS logging
)

add_e2e_test(
Expand Down Expand Up @@ -1465,6 +1468,7 @@ if(BUILD_TESTS)
PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/logging_cert_locust.py
LABEL perf
CONFIGURATIONS perf
BUILD_TARGETS logging
ADDITIONAL_ARGS
--package
"samples/apps/logging/logging"
Expand All @@ -1491,6 +1495,7 @@ if(BUILD_TESTS)
PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/basicperf_locust.py
LABEL perf
CONFIGURATIONS perf
BUILD_TARGETS js_generic
ADDITIONAL_ARGS
--js-app-bundle
${CMAKE_SOURCE_DIR}/samples/apps/basic/js
Expand Down Expand Up @@ -1520,6 +1525,7 @@ if(BUILD_TESTS)
PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/basicperf_locust.py
LABEL perf
CONFIGURATIONS perf
BUILD_TARGETS basic
ADDITIONAL_ARGS
--package
"samples/apps/basic/basic"
Expand Down Expand Up @@ -1551,6 +1557,7 @@ if(BUILD_TESTS)
PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/logging_jwt_locust.py
LABEL perf
CONFIGURATIONS perf
BUILD_TARGETS logging

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems like a roundabout way of defining this? We don't have clean ways to enable/disable individual tests, and yet this is trying to say if this test is enabled, then ccf_bencher must build logging? I think its simpler for now to remove these custom CMake dependencies, and spell out in the preset that bencher builds logging, basic, js_generic. If we add another, we need to remember to add it, but we get clear CI failures if we forget (we try to run a test that depends on a library that hasn't been built). I'd prefer to be even more explicit, and have the tests be their own targets that could be built (and would build their required libs) with the same filtering as when we run them. But CMake doesn't support that, afaict.

ADDITIONAL_ARGS
--package
"samples/apps/logging/logging"
Expand All @@ -1576,6 +1583,7 @@ if(BUILD_TESTS)
PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/historical_query_locust.py
LABEL perf
CONFIGURATIONS perf
BUILD_TARGETS logging
ADDITIONAL_ARGS
--package
"samples/apps/logging/logging"
Expand Down
28 changes: 28 additions & 0 deletions CMakePresets.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"version": 3,
"cmakeMinimumRequired": {
"major": 3,
"minor": 21,
"patch": 0
},
"configurePresets": [
{
"name": "bencher",
"displayName": "Bencher",
"description": "Configure the microbenchmark and end-to-end performance test build.",
"generator": "Ninja",
"binaryDir": "${sourceDir}/build",
"cacheVariables": {
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
"WORKER_THREADS": "2"
}
Comment on lines +15 to +18
}
],
"buildPresets": [
{
"name": "bencher",
"configurePreset": "bencher",
"targets": ["ccf_bencher"]
}
]
}
13 changes: 12 additions & 1 deletion cmake/common.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,24 @@ function(add_e2e_test)
PARSED_ARGS
"DETECT_DEADLOCKS"
"NAME;PYTHON_SCRIPT;LABEL;CURL_CLIENT;BUCKET;TSAN_SUPPRESSIONS"
"CONSTITUTION;ADDITIONAL_ARGS;CONFIGURATIONS"
"CONSTITUTION;ADDITIONAL_ARGS;CONFIGURATIONS;BUILD_TARGETS"
)

if(NOT PARSED_ARGS_CONSTITUTION)
set(PARSED_ARGS_CONSTITUTION ${CCF_NETWORK_TEST_DEFAULT_CONSTITUTION})
endif()

if(BUILD_END_TO_END_TESTS)
if("${PARSED_ARGS_LABEL}" STREQUAL "perf")
if(NOT PARSED_ARGS_BUILD_TARGETS)
message(
FATAL_ERROR
"Perf test ${PARSED_ARGS_NAME} must specify BUILD_TARGETS"
)
endif()
add_dependencies(ccf_bencher ${PARSED_ARGS_BUILD_TARGETS})
endif()

set(PYTHON_WRAPPER ${PYTHON})

# For fast e2e runs, tick node faster than default value (except for
Expand Down Expand Up @@ -262,6 +272,7 @@ function(add_picobench name)
)

add_executable(${name} ${PARSED_ARGS_SRCS})
add_dependencies(ccf_bencher ${name})

target_include_directories(${name} PRIVATE src ${PARSED_ARGS_INCLUDE_DIRS})

Expand Down
2 changes: 1 addition & 1 deletion cmake/gersemi_definitions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function(add_e2e_test)
PARSED_ARGS
"DETECT_DEADLOCKS"
"NAME;PYTHON_SCRIPT;LABEL;CURL_CLIENT;BUCKET;TSAN_SUPPRESSIONS"
"CONSTITUTION;ADDITIONAL_ARGS;CONFIGURATIONS"
"CONSTITUTION;ADDITIONAL_ARGS;CONFIGURATIONS;BUILD_TARGETS"
)
endfunction()

Expand Down