Improve lookup speed of GeoHandler (issue #1291)

DDCore/include/DD4hep/GeoHandler.h

@@ -88,7 +88,12 @@ namespace dd4hep {
 
     protected:
       bool  m_propagateRegions { false };
+
+      /// actual container with std::vector (preserves order)
       std::map<int, std::vector<const TGeoNode*> >*    m_data      { nullptr };
+      /// redundant container with std::set (for lookup purpose)
+      std::map<int, std::set<const TGeoNode*> >* m_set_data { nullptr };
+
       std::map<const TGeoNode*, std::vector<TGeoNode*> >* m_daughters { nullptr };
       /// Internal helper to collect geometry information from traversal
       GeoHandler& i_collect(const TGeoNode* parent,
@@ -109,6 +114,7 @@ namespace dd4hep {
       GeoHandler();
       /// Initializing constructor
       GeoHandler(std::map<int, std::vector<const TGeoNode*> >* ptr,
+		 std::map<int, std::set<const TGeoNode*> >* ptr_set,
                  std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus = nullptr);
       /// Default destructor
       virtual ~GeoHandler();

DDCore/src/GeoHandler.cpp

@@ -55,25 +55,39 @@ namespace {
 /// Default constructor
 detail::GeoHandler::GeoHandler()  {
   m_data = new std::map<int, std::vector<const TGeoNode*> >();
+  m_set_data = new std::map<int, std::set<const TGeoNode*> >();
 }
 
 /// Initializing constructor
 detail::GeoHandler::GeoHandler(std::map<int, std::vector<const TGeoNode*> >* ptr,
-                               std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus)
-  : m_data(ptr), m_daughters(daus)
+                std::map<int, std::set<const TGeoNode*> >* ptr_set,
+		std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus)
+  : m_data(ptr), m_set_data(ptr_set), m_daughters(daus)
 {
 }
 
 /// Default destructor
 detail::GeoHandler::~GeoHandler() {
   if (m_data)
     delete m_data;
+  if (m_set_data)
+    delete m_set_data;
+
   m_data = nullptr;
+  m_set_data = nullptr;
 }
 
 std::map<int, std::vector<const TGeoNode*> >* detail::GeoHandler::release() {
+  /// release the std::vector geometry container (preserves order)
   std::map<int, std::vector<const TGeoNode*> >* d = m_data;
   m_data = nullptr;
+
+  /// the std::set container (for lookup purpose) is not needed anymore, so delete it
+  /// the container is always present since the call of the constructor
+  /// we never expect to call release() twice (will release nullptr)
+  delete m_set_data;
+  m_set_data = nullptr;
+
   return d;
 }
 
@@ -88,13 +102,15 @@ detail::GeoHandler& detail::GeoHandler::collect(DetElement element) {
   DetElement par = element.parent();
   TGeoNode*  par_node = par.isValid() ? par.placement().ptr() : nullptr;
   m_data->clear();
+  m_set_data->clear();
   return i_collect(par_node, element.placement().ptr(), 0, Region(), LimitSet());
 }
 
 detail::GeoHandler& detail::GeoHandler::collect(DetElement element, GeometryInfo& info) {
   DetElement par = element.parent();
   TGeoNode* par_node = par.isValid() ? par.placement().ptr() : nullptr;
   m_data->clear();
+  m_set_data->clear();
   i_collect(par_node, element.placement().ptr(), 0, Region(), LimitSet());
   for ( auto i = m_data->rbegin(); i != m_data->rend(); ++i ) {
     const auto& mapped = (*i).second;
@@ -150,8 +166,8 @@ detail::GeoHandler& detail::GeoHandler::i_collect(const TGeoNode* /* parent */,
     }
   }
   /// Collect the hierarchy of placements
-  auto& vec = (*m_data)[level];
-  if(std::find(vec.begin(), vec.end(), current) == vec.end()) {
+  /// perform lookup using std::set::emplace (faster than std::find for very large number of volumes)
+  if ( (*m_set_data)[level].emplace(current).second ) {
     (*m_data)[level].push_back(current);
   }
   int num = nodes ? nodes->GetEntriesFast() : 0;

DDG4/src/Geant4Converter.cpp

@@ -1668,6 +1668,7 @@ Geant4Converter& Geant4Converter::create(DetElement top) {
   World wrld = top.world();
 
   m_data->clear();
+  m_set_data->clear();
   m_daughters = &daughters;
   geo.manager = &wrld.detectorDescription().manager();
   this->collect(top, geo);