diff --git a/DDCore/include/DD4hep/GeoHandler.h b/DDCore/include/DD4hep/GeoHandler.h index 31d507f17..85031a171 100644 --- a/DDCore/include/DD4hep/GeoHandler.h +++ b/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 >* m_data { nullptr }; + /// redundant container with std::set (for lookup purpose) + std::map >* m_set_data { nullptr }; + std::map >* 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 >* ptr, + std::map >* ptr_set, std::map >* daus = nullptr); /// Default destructor virtual ~GeoHandler(); diff --git a/DDCore/src/GeoHandler.cpp b/DDCore/src/GeoHandler.cpp index ca443d062..07f7a4521 100644 --- a/DDCore/src/GeoHandler.cpp +++ b/DDCore/src/GeoHandler.cpp @@ -55,12 +55,14 @@ namespace { /// Default constructor detail::GeoHandler::GeoHandler() { m_data = new std::map >(); + m_set_data = new std::map >(); } /// Initializing constructor detail::GeoHandler::GeoHandler(std::map >* ptr, - std::map >* daus) - : m_data(ptr), m_daughters(daus) + std::map >* ptr_set, + std::map >* daus) + : m_data(ptr), m_set_data(ptr_set), m_daughters(daus) { } @@ -68,12 +70,24 @@ detail::GeoHandler::GeoHandler(std::map >* ptr 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 >* detail::GeoHandler::release() { + /// release the std::vector geometry container (preserves order) std::map >* 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,6 +102,7 @@ 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()); } @@ -95,6 +110,7 @@ detail::GeoHandler& detail::GeoHandler::collect(DetElement element, GeometryInfo 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; diff --git a/DDG4/src/Geant4Converter.cpp b/DDG4/src/Geant4Converter.cpp index 3b8a6d5e5..57531caf7 100644 --- a/DDG4/src/Geant4Converter.cpp +++ b/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);