Merge pull request #29956 from Dr15Jones/threadSafeDDName

Allow DDNames to be created concurrently
cms-sw · May 28, 2020 · 47fd0ae · 47fd0ae
2 parents 085730d + 3d9aeba
commit 47fd0ae
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 21 deletions.
diff --git a/DetectorDescription/Core/BuildFile.xml b/DetectorDescription/Core/BuildFile.xml
@@ -4,6 +4,7 @@
 <use name="boost"/>
 <use name="clhep"/>
 <use name="rootmath"/>
+<use name="tbb"/>
 <export>
   <lib name="1"/>
 </export>
diff --git a/DetectorDescription/Core/interface/DDName.h b/DetectorDescription/Core/interface/DDName.h
@@ -2,10 +2,10 @@
 #define DETECTOR_DESCRIPTION_CORE_DDNAME_H
 
 #include <iosfwd>
-#include <map>
 #include <string>
 #include <utility>
-#include <vector>
+#include <tbb/concurrent_vector.h>
+#include <tbb/concurrent_unordered_map.h>
 
 class DDCurrentNamespace;
 
@@ -15,8 +15,8 @@ class DDCurrentNamespace;
 class DDName {
 public:
   using id_type = int;
-  using Registry = std::map<std::pair<std::string, std::string>, id_type>;
-  using IdToName = std::vector<Registry::const_iterator>;
+  using Registry = tbb::concurrent_unordered_map<std::pair<std::string, std::string>, id_type>;
+  using IdToName = tbb::concurrent_vector<Registry::const_iterator>;
 
   //! Constructs a DDName with name \a name and assigns \a name to the namespace \a ns.
   DDName(const std::string& name, const std::string& ns);
@@ -47,7 +47,7 @@ class DDName {
 private:
   id_type id_;
 
-  static Registry::iterator registerName(const std::pair<std::string, std::string>& s);
+  static Registry::const_iterator registerName(const std::pair<std::string, std::string>& s);
 };
 
 std::ostream& operator<<(std::ostream& os, const DDName& n);

diff --git a/DetectorDescription/Core/src/DDName.cc b/DetectorDescription/Core/src/DDName.cc
@@ -6,6 +6,7 @@
 #include <ext/alloc_traits.h>
 #include <cstdlib>
 #include <sstream>
+#include <mutex>
 
 std::ostream& operator<<(std::ostream& os, const DDName& n) {
   os << n.ns() << ':' << n.name();
@@ -59,19 +60,28 @@ const std::string& DDName::ns() const {
   return *result;
 }
 
-DDName::Registry::iterator DDName::registerName(const std::pair<std::string, std::string>& nm) {
-  Registry& reg = DDI::Singleton<Registry>::instance();
-  IdToName& idToName = DDI::Singleton<IdToName>::instance();
-  Registry::size_type sz = reg.size();
-  if (!sz) {
-    reg[std::make_pair(std::string(""), std::string(""))] = 0;
+namespace {
+  std::once_flag s_once;
+}  // namespace
+
+DDName::Registry::const_iterator DDName::registerName(const std::pair<std::string, std::string>& nm) {
+  std::call_once(s_once, []() {
+    Registry& reg = DDI::Singleton<Registry>::instance();
+    IdToName& idToName = DDI::Singleton<IdToName>::instance();
+    reg.emplace(std::make_pair(std::string(""), std::string("")), 0);
     idToName.emplace_back(reg.begin());
-    ++sz;
-  }
-  Registry::value_type val(nm, sz);
-  std::pair<Registry::iterator, bool> result = reg.insert(val);
-  if (result.second) {
-    idToName.emplace_back(result.first);
+  });
+  Registry& reg = DDI::Singleton<Registry>::instance();
+
+  Registry::const_iterator itFound = reg.find(nm);
+  if (itFound == reg.end()) {
+    //If two threads are concurrently adding the same name we will get
+    // two entries in IdToName but they will both point to the same entry
+    // to Registry where the first emplace to Registry will set the ID number.
+    IdToName& idToName = DDI::Singleton<IdToName>::instance();
+    auto it = idToName.emplace_back(reg.end());
+    *it = reg.emplace(nm, it - idToName.begin()).first;
+    itFound = *it;
   }
-  return result.first;
+  return itFound;
 }
diff --git a/DetectorDescription/Core/src/StoresInstantiation.cc b/DetectorDescription/Core/src/StoresInstantiation.cc
@@ -17,6 +17,8 @@
 #include <string>
 #include <utility>
 #include <vector>
+#include <tbb/concurrent_vector.h>
+#include <tbb/concurrent_unordered_map.h>
 
 template class DDI::Singleton<AxesNames>;
 template class DDI::Singleton<DDRoot>;
@@ -31,6 +33,8 @@ template class DDI::Singleton<DDI::Store<DDName, std::unique_ptr<DDI::Solid> > >
 template class DDI::Singleton<DDI::Store<DDName, std::unique_ptr<double> > >;
 template class DDI::Singleton<DDI::Store<DDName, std::unique_ptr<DDRotationMatrix> > >;
 template class DDI::Singleton<DDI::Store<DDName, std::unique_ptr<DDI::Division>, std::unique_ptr<DDI::Division> > >;
-template class DDI::Singleton<std::map<std::pair<std::string, std::string>, int> >;
-template class DDI::Singleton<std::map<std::string, std::vector<DDName> > >;
-template class DDI::Singleton<std::vector<std::map<std::pair<std::string, std::string>, int>::const_iterator> >;
+template class DDI::Singleton<std::map<std::string, std::vector<DDName> > >;  //Used internally by DDLogicalPart
+//The following are used by DDName
+template class DDI::Singleton<tbb::concurrent_unordered_map<std::pair<std::string, std::string>, int> >;
+template class DDI::Singleton<
+    tbb::concurrent_vector<tbb::concurrent_unordered_map<std::pair<std::string, std::string>, int>::const_iterator> >;