Problem 1581

Summary: GetCollectionID very slow - obvious bug in HCTable
Product: Geant4 Reporter: Laurie Nevay <laurie.nevay>
Component: digits_hitsAssignee: asai
Status: RESOLVED WONTFIX    
Severity: minor CC: jochem.snuverink, laurie.nevay
Priority: P5    
Version: 10.0   
Hardware: All   
OS: All   

Description Laurie Nevay 2014-03-04 11:46:09 CET 
G4HCtable::GetCollectionID (line46 of G4HCtable.cc) is very slow. For simulations with aggressive cuts and a large number of sensitive detectors, this function dominates the cpu time on the first event.  

G4HCtable.cc is used by G4SDManager to (sic) registor hits collections.  
HCtable stores this information as two private member vectors of G4string.

Line46 G4int G4HCtable::GetCollectionID(G4String HCname) const

This could be made faster by perhaps using std::map or std::strcmp.  You could also sort the vector and boolean compare it.  I'm not a pro, but this method dominates the first event.

Ideally you would get the hits collection id back when you create it and then you wouldn't have to search through all of them to find the id. 

Thanks!
Comment 1 Laurie Nevay 2014-03-04 12:22:18 CET 
Also, the for loop in the algorithm does not break when it finds a match - it continues to iterate through the vector.

HCtable.cc line51

for(size_t j=0;j<HClist.size();j++)
    {
      if(HClist[j]==HCname)
      { 
        if(i>=0) return -2; //so only break if there's another with the same name
        i = j;  //should break here
      }

So if the intention is to search through the whole vector to make sure there's not another with the same name, the safety from this is quite costly.  If we keep this safety check, this should perhaps be in the register process.
Comment 2 asai 2014-03-04 17:33:28 CET 
It is a surprise to me that you reported this method dominated the processing time of the first event. How many sensitive detectors do you have?
Makoto
Comment 3 Laurie Nevay 2014-03-06 13:40:25 CET 
Hello,

We have between 10k and 40k sensitive detectors.  Usually much fewer, but at this level, this became quite apparent as the cause of the slow down.  I can certainly imagine our implementation isn't the best, but this part certainly seems to follow the examples I've seen.

In our sensitive detector class we have:

void BDSEnergyCounterSD::Initialize(G4HCofThisEvent*HCE)
{
  BDSEnergyCounterCollection = new BDSEnergyCounterHitsCollection
    (SensitiveDetectorName,collectionName[0]); 
  if (itsHCID < 0){
    itsHCID = G4SDManager::GetSDMpointer()->GetCollectionID(collectionName[0]);}
//    itsHCID = G4SDManager::GetSDMpointer()->GetCollectionID(BDSEnergyCounterCollection);} //slow version
  HCE->AddHitsCollection(itsHCID,BDSEnergyCounterCollection);
}
Comment 4 asai 2014-04-14 20:33:58 CEST 
Equivalent to #1588.