Skip to content

Replace deprecated ToolHandle constuctor in MergedPi0

Christoph Hasse requested to merge chasse_fix into master

This MR is motivated by the attempt to try and fix the problem that it seems like reserveIOV is taking the most time in the lead lead tests.

From what I understand the following happens: During the start() of reserveIOV we create the first conditions interval of validity, but it seems that this IOV is directly invalid on the first event. Not sure why, I didn't dig much deeper... @clemenci is that supposed to be the case?
When the processing starts we have 20 threads which all start running, the first thread to enter reserveIOV is the one that creates a new interval of validity while the other 19 wait for this to happen.

Once that is done, all 20 threads continue processing. The first thread to get to the calo algo CaloFutureMergedPi0 will now trigger the initialization of some tools which this algorithm uses, as that hasn't happened for tools that use the old constructor.
But the registerCondition in the tool initialize will also invalidate the current interval of validity.
This means the next event to start and enter reserveIOV will try and create a new interval of validity, but for this to happen, all events that started on the old one will first need to finish.

Problem: So now we have 19 threads that first need to finish before the one in reserveIOV can acquire the lock and continue. That's why we spend so much time in the operator of reserveIOV

Using the modern ToolHandle constructor means the registerCondition call happens during the initialization of the application, and the reserveIOV of the first event creates an interval of validity that will be valid for all events.
This should then also "fix" what is reported in the throughput test.

BUT NOTE AFAIK this can only fix the throughput test. In the real world, we will observe the same thing when we change conditions. Usually this wait time of having all threads with old events finish before we start the events with new conditions isn't really problematic as events are quickly processed, but I guess for lead lead this assumption doesn't really hold.

Will we be able to have multiple conditions in flight when using DD4HEP?

Related to lhcb-datapkg/PRConfig!201 (merged) and Moore!1164 (merged)

cc @baudurie @gunther @sstahl @rmatev

Edited by Rosen Matev

Merge request reports