Add type to PrepRawData, move implementations .icc, avoid dyn_cast

• Add a type function to PrepRawData
• Move a few inline to separate .icc
• Use it to avoid a couple of dynamic_casts

This merge request affects 5 packages:

• InnerDetector/InDetRecEvent/InDetPrepRawData
• InnerDetector/InDetRecTools/InDetConversionFinderTools
• MuonSpectrometer/MuonReconstruction/MuonRecEvent/MuonPrepRawData
• Simulation/ISF/ISF_Fatras/ISF_FatrasEvent
• Tracking/TrkEvent/TrkPrepRawData

Adding @goetz ,@jchapman ,@amorley ,@wleight ,@vpascuzz ,@sroe ,@nkoehler ,@rosati as watchers

added Egamma InnerDetector MuonSpectrometer Simulation Tracking master review-pending-level-1 labels

CI Result FAILURE (hash 4f4aab1a)

	Athena	AthSimulation	AthGeneration
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 17544]

Hmm. It's fine to add the type, but is there a piece of code which you are trying to optimise? Because I don't think dynamic_cast is that slow these days.

The one place where is might be slow is a bunch of dynamic_casts in switches. But then you should have a complete list of PRD types, no? (I added a comment about this in the code)

dynamic_cast is that slow these days.

Actually not sure , @goetz was claiming the same but it turns out the above can be 2X faster or so. My understanding from dissasembing is that can stil call strcmp.

On why, to be honest to do know why we have that many (seems like a part of the design gone wrong), for the Measurement side there is a type we can use . There is also now a rio_type specifc etc.

Now the provenance of the calls is not obvious : https://atlaspmb.web.cern.ch/atlaspmb/arch-mon-rawtoall_tier0_reco_data17/profiling_reports/2020-06-17T2138/profile_2020-06-17T2138.pdf

But I think @amete also pointed out that prb comes from loops inside tracking looking for a a specific type in tight loops. Basically is more like trying to figure why dynamic_cast has same cost as sincos or getCache in our profile and cut it down.

In terms of code the things I have seem are like tracking summary or ID quantities. Basically, if you see what I changes as an example, the code just wants to count number of nSi vs nTRT and goes into a loop per track, with dynamic casts ....

Anyhow the kind of example is this

    if (rd && rd->type(Trk::PrepRawDataType::SiCluster)) {
        ++nclus;
        continue;
      }

      if (rd && rd->type(Trk::PrepRawDataType::TRT_DriftCircle)) {
        ++ntrt;
        continue;
      }

instead of

  const InDet::SiCluster*     si = dynamic_cast<const InDet::SiCluster*>(rd);	       
      if(si) { ++nclus; continue;}							       
      
      const InDet::TRT_DriftCircle*  dc = dynamic_cast<const InDet::TRT_DriftCircle*>(rd); 
      if(dc) { ++ntrt; continue;}			

from this MR.

At large looking for dynamic_casts inside tight loops.

The switch on type another issue , but that we do more on surface/Parameter type rather than on things like the above. Since certain tranformation implementation live outside the classes.

This was another set of MRs multiple if else if .... based on dynamic cast or so to kind od switch or if else over types .

added 1 commit

• 432bd757 - indent SiCluster.h

Compare with previous version

This merge request affects 5 packages:

• InnerDetector/InDetRecEvent/InDetPrepRawData
• InnerDetector/InDetRecTools/InDetConversionFinderTools
• MuonSpectrometer/MuonReconstruction/MuonRecEvent/MuonPrepRawData
• Simulation/ISF/ISF_Fatras/ISF_FatrasEvent
• Tracking/TrkEvent/TrkPrepRawData

Adding @goetz ,@jchapman ,@amorley ,@wleight ,@vpascuzz ,@sroe ,@nkoehler ,@rosati as watchers

resolved all threads

added 1 commit

• 730fc304 - As many types , and as many overloads as detectorElement overloads

Compare with previous version

resolved all threads

Now as many types and as many overloads of type(...) as detectorElement() overloads I could see. As I assume this is a logical way to approach it.

added 1 commit

• 5a5ac385 - As many types , and as many overloads as detectorElement overloads

Compare with previous version

This merge request affects 5 packages:

• InnerDetector/InDetRecEvent/InDetPrepRawData
• InnerDetector/InDetRecTools/InDetConversionFinderTools
• MuonSpectrometer/MuonReconstruction/MuonRecEvent/MuonPrepRawData
• Simulation/ISF/ISF_Fatras/ISF_FatrasEvent
• Tracking/TrkEvent/TrkPrepRawData

Adding @goetz ,@jchapman ,@amorley ,@wleight ,@vpascuzz ,@sroe ,@nkoehler ,@rosati as watchers

This merge request affects 5 packages:

• InnerDetector/InDetRecEvent/InDetPrepRawData
• InnerDetector/InDetRecTools/InDetConversionFinderTools
• MuonSpectrometer/MuonReconstruction/MuonRecEvent/MuonPrepRawData
• Simulation/ISF/ISF_Fatras/ISF_FatrasEvent
• Tracking/TrkEvent/TrkPrepRawData

Adding @goetz ,@jchapman ,@amorley ,@wleight ,@vpascuzz ,@sroe ,@nkoehler ,@rosati as watchers

CI Result FAILURE (hash 5a5ac385)

	Athena	AthSimulation	AthGeneration
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 17613]

Jenkins please retry a build

This merge request affects 5 packages:

• InnerDetector/InDetRecEvent/InDetPrepRawData
• InnerDetector/InDetRecTools/InDetConversionFinderTools
• MuonSpectrometer/MuonReconstruction/MuonRecEvent/MuonPrepRawData
• Simulation/ISF/ISF_Fatras/ISF_FatrasEvent
• Tracking/TrkEvent/TrkPrepRawData

Adding @goetz ,@jchapman ,@amorley ,@wleight ,@vpascuzz ,@sroe ,@nkoehler ,@rosati as watchers