Add BenchmarkSystem that implements benchmarking #52
Conversation
marcin-wierzbicki
force-pushed
the
cr-124398
branch
from
c4857f0 to
e6648b5
Compare
|
I fixed minor issue with printing percentages: |
marcin-wierzbicki
force-pushed
the
cr-124398
branch
from
e6648b5 to
5291159
Compare
|
|
||
| BenchmarkSystem::BenchmarkSystem( | ||
| ::async::ContextType const context, | ||
| ::lifecycle::ILifecycleManager& lifecycleManager, |
| #include <mcu/mcu.h> | ||
| #include <runtime/StatisticsContainer.h> | ||
|
|
||
| #define GET_HW_COUNTER() (DWT->CYCCNT) |
There was a problem hiding this comment.
I guess this is defined as a macro just to clarify that "CYCCNT" is a HW counter? It would be more common to not have a macro, but maybe just a comment above the first usage telling what it is. This habit originates from Misra that discourages using function-like macros.
| src/main.cpp) | ||
|
|
||
| if (BUILD_BENCHMARK) | ||
| target_sources(app.referenceApp PRIVATE src/systems/BenchmarkSystem.cpp) |
There was a problem hiding this comment.
Should we maybe add a check here that prevents the benchmarking build on Posix? This might be slightly out of place, but currently it will fail due to a compiler error which is also not good. So you could just add:
if (BUILD_TARGET_PLATFORM STREQUAL "POSIX") message(FATAL_ERROR "Benchmarking not supported on POSIX") endif ()
| @@ -0,0 +1,10 @@ | |||
| // Copyright 2024 Accenture. | |||
|
|
|||
| #ifndef GUARD_A2388DF9_4B35_42D5_8B75_DFC18C21C372 | |||
There was a problem hiding this comment.
We should use "#pragma once" now
| ::estd::typed_mem<::systems::DemoSystem> demoSystem; | ||
| #else | ||
| ::estd::typed_mem<::systems::BenchmarkSystem> benchmarkSystem; | ||
| #endif |
There was a problem hiding this comment.
Please do some regrouping to keep this clearer, eg:
#ifdef BENCHMARK
::estd::typed_mem<::systems::BenchmarkSystem> benchmarkSystem;
#else
::estd::typed_mem<::systems::RuntimeSystem> runtimeSystem;
::estd::typed_mem<::systems::DemoSystem> demoSystem;
#endif
::estd::typed_mem<::systems::SysAdminSystem> sysAdminSystem;
::estd::typed_mem<::systems::SafetySystem> safetySystem;
|
|
||
| void enterTask(size_t const taskIdx) | ||
| { | ||
| #ifdef BENCHMARK |
There was a problem hiding this comment.
Ifdefs make the code less testable so I think we should use them sparingly in the common code under libs/bsw. Since this file isn't even 200 lines long, maybe you could just create a copy like RuntimeMonitorSimple.h instead of modifying the original?
doc/learning/benchmark/index.rst
Outdated
| The reference app can be built to run benchmark tests. There is a BenchmarkSystem class that | ||
| implements the following tests: | ||
|
|
||
| * interrupt latency test - to measure the time between raising the interrupt line and entering the ISR, |
There was a problem hiding this comment.
Bullet point lists don't need commas at the end of entries
marcin-wierzbicki
force-pushed
the
cr-124398
branch
2 times, most recently
from
ec827d2 to
1b7f114
Compare
|
@hbragge-acn : addressed review comments |
marcin-wierzbicki
force-pushed
the
cr-124398
branch
from
1b7f114 to
9c1e347
Compare
Added these benchmark tests: - interrupt latency test, - task switch latency test, - task switch after timeout latency test, - load benchmark test. Signed-off-by: Marcin Wierzbicki <[email protected]>
marcin-wierzbicki
force-pushed
the
cr-124398
branch
from
9c1e347 to
aad8478
Compare
|
fixed code formatting |
3 participants
originates from CR #124398