Improving calo correction code (Follow-up from "Faster calo correction (CaloFutureShowerOverlap)")
The following discussions from !1833 (merged) should be addressed:
-
@ahennequ started a discussion: (+9 comments) Moving this function from .cpp to .h and changing the derivative from ptr to reference would allow the compiler to inline the function and decide by itself if the derivative should be computed (based on whether it is used or not). Effectively moving the test
doDerivative
from runtime to compile time.https://devblogs.microsoft.com/cppblog/optimizing-c-code-dead-code-elimination/
-
@ahennequ started a discussion: (+1 comment) It seems like it would be possible to have a compile time dispatching of functions. Effectively, this would mean spliting this big function into one function per
CaloFutureCorrection::Type
and calling the type specific function instead of passing the type in parameter.This would allow to remove a lot of branching, the optional (the only null return value is if the type is unknown), the checks for number of parameters (this would be the arguments of the function), would generate less code and be more readable.
One caveat is that we loose the ability to change at configuration time the mapping type->function, but is it really needed ?
@sponce FYI
-
@ahennequ started a discussion: (+4 comments) This check (and all the temp.size() checks in other else if cases) can be done at initialisation time (or when updating m_params from conditions).
-
@ahennequ started a discussion: (+8 comments) this can be replaced by and std::array
-
@ahennequ started a discussion: (+4 comments) can the area be given directly instead of constructing a fake cellID ?
Apart from this, since CaloFutureshowerOverlapTool::process()
is called by CaloFiutureShowerOverlap::operator()
for each pair of clusters, there could be some overhead because of the tool. Maybe the code could be simplified somehow, or the tool could be called once per event?