Skip to content

Commit bc9835b

Browse files
authored
[QC-1346] Scaffolding to share common code among QC data processors (#2660)
* QC-1346 Scaffolding to share common code among QC data processors Actor is a template base class for all QC Data Processors. It is supposed to bring their commonalities together, such as: service initialization, Data Processing Layer adoption, retrieving configuration and runtime parameters, interactions with controlling entities (DPL driver, AliECS, ODC). The design is based on CRTP (see the web for explanation), which allows us to: - avoid code repetition in implementing aforementioned commonalities - optionally perform certain actions depending on traits of an Actor specialization. CRTP, in contrast to dynamic inheritance, is also advertised to avoid performance impact due to vtable lookups. It is certainly a nice bonus in our case, but it was not the main motivation for CRTP-based approach. To allow for compile-time customization of centralized Actor features, we require each concrete Actor to implement an ActorTraits structure with certain parameters which is enforced with ValidActorTraits concept. The traits are separated from the main Actor class to improve readability and allow for shorter compilation times by allowing many helper functions avoid including Actor.h and a corresponding actor specialization. For additional savings on compilation time and clutter in code, we validate ActorTraits specializations with a concept only in Actor, but this could be revisited if proven wrong. This commit paves the path for refactoring existing QC data processors as Actor specializations/children. * keep clang happy even though I don't believe it's right Clang complains that: ``` SOURCES/QualityControl/2660/0/Framework/test/testActor.cxx /Volumes/build/alice-ci-workdir/qualitycontrol-o2/sw/SOURCES/QualityControl/2660/0/Framework/test/testActor.cxx:97:5: error: call to consteval function 'o2::quality_control::core::UnrequestedAccessActor::assertNoAccessToServices<o2::quality_control::core::UnrequestedAccessActor>' is not a constant expression 97 | assertNoAccessToServices<UnrequestedAccessActor>(); | ^ /Volumes/build/alice-ci-workdir/qualitycontrol-o2/sw/SOURCES/QualityControl/2660/0/Framework/test/testActor.cxx:97:5: note: implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function 1 error generated. ``` GCC is fine and is able to process it as a consteval expression. Let's keep both happy by using constexpr, given it's just a test. * Assume "resilient" for critical data processors with uncertain upstream * Move correct inheritance check to happen after the type is fully known to make clang happy again * Get rif of the protection against faulty inheritance of actors The disadvantage is that then the child can access and modify parent's private members, such as mActivity, and cause rather unpredictable behaviours. I don't think it's worth having a protection which in the end would cause a different kind of bugs. * add missing file header
1 parent db0550b commit bc9835b

22 files changed

Lines changed: 1997 additions & 17 deletions

Framework/CMakeLists.txt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ add_library(O2QualityControl
140140
src/QCInputsAdapters.cxx
141141
src/QCInputsFactory.cxx
142142
src/UserInputOutput.cxx
143+
src/Actor.cxx
144+
src/ActorHelpers.cxx
145+
src/DataProcessorAdapter.cxx
143146
)
144147

145148
target_include_directories(
@@ -270,12 +273,16 @@ endforeach()
270273
add_executable(o2-qc-test-core
271274
test/testActivity.cxx
272275
test/testActivityHelpers.cxx
276+
test/testActorHelpers.cxx
277+
test/testActorTraits.cxx
278+
test/testActor.cxx
273279
test/testAggregatorInterface.cxx
274280
test/testAggregatorRunner.cxx
275281
test/testCheck.cxx
276282
test/testCheckInterface.cxx
277283
test/testCheckRunner.cxx
278284
test/testCustomParameters.cxx
285+
test/testDataProcessorAdapter.cxx
279286
test/testDataHeaderHelpers.cxx
280287
test/testInfrastructureGenerator.cxx
281288
test/testMonitorObject.cxx
@@ -295,6 +302,7 @@ add_executable(o2-qc-test-core
295302
test/testQualitiesToFlagCollectionConverter.cxx
296303
test/testQCInputs.cxx
297304
test/testUserInputOutput.cxx
305+
test/testStringUtils.cxx
298306
)
299307
set_property(TARGET o2-qc-test-core
300308
PROPERTY RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/tests)
@@ -310,6 +318,7 @@ target_include_directories(o2-qc-test-core PRIVATE ${CMAKE_SOURCE_DIR})
310318
target_include_directories(o2-qc-test-core PRIVATE $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>)
311319

312320
set(TEST_SRCS
321+
test/testActorCallbacks.cxx
313322
test/testDbFactory.cxx
314323
test/testPublisher.cxx
315324
test/testQcInfoLogger.cxx
@@ -325,12 +334,12 @@ set(TEST_SRCS
325334
test/testWorkflow.cxx
326335
test/testRepoPathUtils.cxx
327336
test/testUserCodeInterface.cxx
328-
test/testStringUtils.cxx
329337
test/testRunnerUtils.cxx
330338
test/testBookkeepingQualitySink.cxx
331339
)
332340

333341
set(TEST_ARGS
342+
"-b --run"
334343
""
335344
""
336345
""
@@ -380,6 +389,8 @@ endforeach()
380389

381390
target_include_directories(testCcdbDatabase PRIVATE $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/include>)
382391

392+
set_property(TEST testActorCallbacks PROPERTY TIMEOUT 30)
393+
set_property(TEST testActorCallbacks PROPERTY LABELS slow)
383394
set_property(TEST testWorkflow PROPERTY TIMEOUT 40)
384395
set_property(TEST testWorkflow PROPERTY LABELS slow)
385396
set_property(TEST testCheckWorkflow PROPERTY TIMEOUT 50)

0 commit comments

Comments
 (0)