Merge pull request #32544 from ghugo83/dd4hep_perf_test

Improve DD4hep workflow perf, step 4: Improve performance of GeometricDet construction in old DD (DDD) and DD4hep
cms-sw · Dec 22, 2020 · 87119e3 · 87119e3
2 parents d10412c + 0663f49
commit 87119e3
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 46 deletions.
diff --git a/DetectorDescription/DDCMS/src/DDFilteredView.cc b/DetectorDescription/DDCMS/src/DDFilteredView.cc
@@ -770,8 +770,9 @@ const int DDFilteredView::nodeCopyNoAt(int level) const {
 // Compare if name matches a selection pattern that
 // may or may not be defined as a regular expression
 bool DDFilteredView::compareEqualName(const std::string_view selection, const std::string_view name) const {
-  return (!(dd4hep::dd::isRegex(selection)) ? dd4hep::dd::compareEqual(name, selection)
-                                            : regex_match(name.begin(), name.end(), regex(std::string(selection))));
+  return (!(dd4hep::dd::isRegex(selection))
+              ? dd4hep::dd::compareEqual(name, selection)
+              : regex_match(name.begin(), name.end(), regex(selection.begin(), selection.end())));
 }
 
 // Check if both name and it's selection pattern

diff --git a/Geometry/TrackerNumberingBuilder/interface/GeometricDet.h b/Geometry/TrackerNumberingBuilder/interface/GeometricDet.h
@@ -171,28 +171,28 @@ class GeometricDet {
   GeometricEnumType type_;
 
   nav_type ddd_;
-  DetId geographicalID_;
+  DetId geographicalID_ = 0;
 
   Translation trans_;  // in mm
-  double rho_;         // in mm
-  double phi_;
+  double rho_ = 0.;    // in mm
+  double phi_ = 0.;
   RotationMatrix rot_;
 
   cms::DDSolidShape shape_;
   std::vector<double> params_;  // in mm
 
-  double radLength_;
-  double xi_;
-  double pixROCRows_;
-  double pixROCCols_;
-  double pixROCx_;
-  double pixROCy_;
-  bool stereo_;
-  bool isLowerSensor_;
-  bool isUpperSensor_;
-  double siliconAPVNum_;
-
-  bool isFromDD4hep_;
+  double radLength_ = 0.;
+  double xi_ = 0.;
+  double pixROCRows_ = 0.;
+  double pixROCCols_ = 0.;
+  double pixROCx_ = 0.;
+  double pixROCy_ = 0.;
+  bool stereo_ = false;
+  bool isLowerSensor_ = false;
+  bool isUpperSensor_ = false;
+  double siliconAPVNum_ = 0.;
+
+  bool isFromDD4hep_ = false;
 
   ConstGeometricDetContainer container_;
 };

diff --git a/Geometry/TrackerNumberingBuilder/plugins/DDDCmsTrackerContruction.cc b/Geometry/TrackerNumberingBuilder/plugins/DDDCmsTrackerContruction.cc
@@ -39,14 +39,14 @@ std::unique_ptr<GeometricDet> DDDCmsTrackerContruction::construct(const DDCompac
   CmsTrackerBuilder<DDFilteredView> theCmsTrackerBuilder;
   theCmsTrackerBuilder.build(fv, tracker.get(), attribute);
 
-  if (DEBUG) {
-    printAllTrackerGeometricDetsBeforeDetIDBuilding(tracker.get());
-  }
-
   edm::LogVerbatim("DDDCmsTrackerContruction") << "Assign DetIds";
   CmsTrackerDetIdBuilder theCmsTrackerDetIdBuilder(detidShifts);
   theCmsTrackerDetIdBuilder.buildId(*tracker);
 
+  if (DEBUG) {
+    printAllTrackerGeometricDetsBeforeDetIDBuilding(tracker.get());
+  }
+
   fv.parent();
   //
   // set the Tracker
@@ -83,14 +83,14 @@ std::unique_ptr<GeometricDet> DDDCmsTrackerContruction::construct(const cms::DDC
   CmsTrackerBuilder<cms::DDFilteredView> theCmsTrackerBuilder;
   theCmsTrackerBuilder.build(fv, tracker.get(), attribute);
 
-  if (DEBUG) {
-    printAllTrackerGeometricDetsBeforeDetIDBuilding(tracker.get());
-  }
-
   edm::LogVerbatim("DDDCmsTrackerContruction") << "Assign DetIds";
   CmsTrackerDetIdBuilder theCmsTrackerDetIdBuilder(detidShifts);
   theCmsTrackerDetIdBuilder.buildId(*tracker);
 
+  if (DEBUG) {
+    printAllTrackerGeometricDetsBeforeDetIDBuilding(tracker.get());
+  }
+
   return tracker;
 }
 

diff --git a/Geometry/TrackerNumberingBuilder/src/GeometricDet.cc b/Geometry/TrackerNumberingBuilder/src/GeometricDet.cc
@@ -78,20 +78,24 @@ GeometricDet::GeometricDet(DDFilteredView* fv, GeometricEnumType type)
       rot_(fv->rotation()),
       shape_(cms::dd::name_from_value(cms::LegacySolidShapeMap, fv->shape())),
       params_(fv->parameters()),
-      radLength_(getDouble("TrackerRadLength", *fv)),
-      xi_(getDouble("TrackerXi", *fv)),
-      pixROCRows_(getDouble("PixelROCRows", *fv)),
-      pixROCCols_(getDouble("PixelROCCols", *fv)),
-      pixROCx_(getDouble("PixelROC_X", *fv)),
-      pixROCy_(getDouble("PixelROC_Y", *fv)),
-      stereo_(getString("TrackerStereoDetectors", *fv) == strue),
-      isLowerSensor_(getString("TrackerLowerDetectors", *fv) == strue),
-      isUpperSensor_(getString("TrackerUpperDetectors", *fv) == strue),
-      siliconAPVNum_(getDouble("SiliconAPVNumber", *fv)),
       isFromDD4hep_(false) {
   //  workaround instead of this at initialization
   const DDFilteredView::nav_type& nt = fv->navPos();
   ddd_ = nav_type(nt.begin(), nt.end());
+
+  // Only look for sensor-related info on sensor volumes!
+  if (type_ == DetUnit) {
+    radLength_ = getDouble("TrackerRadLength", *fv);
+    xi_ = getDouble("TrackerXi", *fv);
+    pixROCRows_ = getDouble("PixelROCRows", *fv);
+    pixROCCols_ = getDouble("PixelROCCols", *fv);
+    pixROCx_ = getDouble("PixelROC_X", *fv);
+    pixROCy_ = getDouble("PixelROC_Y", *fv);
+    stereo_ = (getString("TrackerStereoDetectors", *fv) == strue);
+    isLowerSensor_ = (getString("TrackerLowerDetectors", *fv) == strue);
+    isUpperSensor_ = (getString("TrackerUpperDetectors", *fv) == strue);
+    siliconAPVNum_ = getDouble("SiliconAPVNumber", *fv);
+  }
 }
 
 /*
@@ -107,18 +111,28 @@ GeometricDet::GeometricDet(cms::DDFilteredView* fv, GeometricEnumType type)
       rot_(fv->rotation()),
       shape_(fv->shape()),
       params_(computeLegacyShapeParameters(shape_, fv->solid())),
-      pixROCRows_(fv->get<double>("PixelROCRows")),
-      pixROCCols_(fv->get<double>("PixelROCCols")),
-      pixROCx_(fv->get<double>("PixelROC_X")),
-      pixROCy_(fv->get<double>("PixelROC_Y")),
-      stereo_(fv->get<std::string_view>("TrackerStereoDetectors") == strue),
-      isLowerSensor_(fv->get<std::string_view>("TrackerLowerDetectors") == strue),
-      isUpperSensor_(fv->get<std::string_view>("TrackerUpperDetectors") == strue),
-      siliconAPVNum_(fv->get<double>("SiliconAPVNumber")),
       isFromDD4hep_(true) {
-  fv->findSpecPar("TrackerRadLength", "TrackerXi");
-  radLength_ = fv->getNextValue("TrackerRadLength");
-  xi_ = fv->getNextValue("TrackerXi");
+  // Only look for sensor-related info on sensor volumes!
+  if (type_ == DetUnit) {
+    // IT sensors only (NB: hence could add a branch here, but not a critical part on perf)
+    pixROCRows_ = fv->get<double>("PixelROCRows");
+    pixROCCols_ = fv->get<double>("PixelROCCols");
+    pixROCx_ = fv->get<double>("PixelROC_X");
+    pixROCy_ = fv->get<double>("PixelROC_Y");
+
+    // Phase 1 OT sensors only (NB: hence could add a branch here, but not a critical part on perf)
+    stereo_ = (fv->get<std::string_view>("TrackerStereoDetectors") == strue);
+    siliconAPVNum_ = fv->get<double>("SiliconAPVNumber");
+
+    // Phase 2 OT sensors only (NB: hence could add a branch here, but not a critical part on perf)
+    isLowerSensor_ = (fv->get<std::string_view>("TrackerLowerDetectors") == strue);
+    isUpperSensor_ = (fv->get<std::string_view>("TrackerUpperDetectors") == strue);
+
+    // All sensors: IT or OT, Phase 1 or Phase 2 (NB: critical part on perf)
+    fv->findSpecPar("TrackerRadLength", "TrackerXi");
+    radLength_ = fv->getNextValue("TrackerRadLength");
+    xi_ = fv->getNextValue("TrackerXi");
+  }
 }
 
 /*