perf: Avoid some reallocations in GSF (#2447)

This avoids reallocation of the component cache each call to the actor. Reduces runtime of the GSF Actor measurable (component convolution: 12% -> 9% of total actor time)
acts-project · Sep 14, 2023 · 4b8222d · 4b8222d
1 parent 1fb3f22
commit 4b8222d
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 33 deletions.
diff --git a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp
@@ -276,7 +276,7 @@ struct GaussianSumFitter {
           CurvilinearTrackParameters, decltype(fwdPropOptions.actionList)>
           inputResult;
 
-      auto& r = inputResult.template get<detail::GsfResult<traj_t>>();
+      auto& r = inputResult.template get<typename GsfActor::result_type>();
 
       r.fittedStates = &trackContainer.trackStateContainer();
 
@@ -301,7 +301,8 @@ struct GaussianSumFitter {
       return return_error_or_abort(fwdResult.error());
     }
 
-    auto& fwdGsfResult = fwdResult->template get<detail::GsfResult<traj_t>>();
+    auto& fwdGsfResult =
+        fwdResult->template get<typename GsfActor::result_type>();
 
     if (!fwdGsfResult.result.ok()) {
       return return_error_or_abort(fwdGsfResult.result.error());
@@ -356,7 +357,7 @@ struct GaussianSumFitter {
               std::declval<decltype(bwdPropOptions)>(),
               std::declval<decltype(inputResult)>()));
 
-      auto& r = inputResult.template get<detail::GsfResult<traj_t>>();
+      auto& r = inputResult.template get<typename GsfActor::result_type>();
 
       r.fittedStates = &trackContainer.trackStateContainer();
 
@@ -387,7 +388,8 @@ struct GaussianSumFitter {
       return return_error_or_abort(bwdResult.error());
     }
 
-    auto& bwdGsfResult = bwdResult->template get<detail::GsfResult<traj_t>>();
+    auto& bwdGsfResult =
+        bwdResult->template get<typename GsfActor::result_type>();
 
     if (!bwdGsfResult.result.ok()) {
       return return_error_or_abort(bwdGsfResult.result.error());

diff --git a/Core/include/Acts/TrackFitting/detail/GsfActor.hpp b/Core/include/Acts/TrackFitting/detail/GsfActor.hpp
@@ -31,7 +31,7 @@
 namespace Acts {
 namespace detail {
 
-template <typename traj_t>
+template <typename traj_t, typename component_cache_t>
 struct GsfResult {
   /// The multi-trajectory which stores the graph of components
   traj_t* fittedStates{nullptr};
@@ -58,6 +58,9 @@ struct GsfResult {
 
   // Propagate potential errors to the outside
   Result<void> result{Result<void>::success()};
+
+  // Internal: component cache to avoid reallocation
+  std::vector<component_cache_t> componentCache;
 };
 
 /// The actor carrying out the GSF algorithm
@@ -66,8 +69,34 @@ struct GsfActor {
   /// Enforce default construction
   GsfActor() = default;
 
+  /// Stores meta information about the components
+  struct MetaCache {
+    /// Where to find the parent component in the MultiTrajectory
+    MultiTrajectoryTraits::IndexType parentIndex = 0;
+
+    /// Other quantities TODO are they really needed here? seems they are
+    /// reinitialized to Identity etc.
+    BoundMatrix jacobian;
+    BoundToFreeMatrix jacToGlobal;
+    FreeMatrix jacTransport;
+    FreeVector derivative;
+
+    /// We need to preserve the path length
+    ActsScalar pathLength = 0;
+  };
+
+  /// Stores parameters of a gaussian component
+  struct ParameterCache {
+    ActsScalar weight = 0;
+    BoundVector boundPars;
+    BoundSquareMatrix boundCov;
+  };
+
+  /// Broadcast Cache Type
+  using ComponentCache = std::tuple<ParameterCache, MetaCache>;
+
   /// Broadcast the result_type
-  using result_type = GsfResult<traj_t>;
+  using result_type = GsfResult<traj_t, ComponentCache>;
 
   // Actor configuration
   struct Config {
@@ -119,38 +148,12 @@ struct GsfActor {
 
   const Logger& logger() const { return *m_cfg.logger; }
 
-  /// Stores meta information about the components
-  struct MetaCache {
-    /// Where to find the parent component in the MultiTrajectory
-    MultiTrajectoryTraits::IndexType parentIndex = 0;
-
-    /// Other quantities TODO are they really needed here? seems they are
-    /// reinitialized to Identity etc.
-    BoundMatrix jacobian;
-    BoundToFreeMatrix jacToGlobal;
-    FreeMatrix jacTransport;
-    FreeVector derivative;
-
-    /// We need to preserve the path length
-    ActsScalar pathLength = 0;
-  };
-
-  /// Stores parameters of a gaussian component
-  struct ParameterCache {
-    ActsScalar weight = 0;
-    BoundVector boundPars;
-    BoundSquareMatrix boundCov;
-  };
-
   struct TemporaryStates {
     traj_t traj;
     std::vector<MultiTrajectoryTraits::IndexType> tips;
     std::map<MultiTrajectoryTraits::IndexType, double> weights;
   };
 
-  /// Broadcast Cache Type
-  using ComponentCache = std::tuple<ParameterCache, MetaCache>;
-
   /// @brief GSF actor operation
   ///
   /// @tparam propagator_state_t is the type of Propagagor state
@@ -301,7 +304,10 @@ struct GsfActor {
         return;
       }
 
-      std::vector<ComponentCache> componentCache;
+      // Reuse memory over all calls to the Actor in a single propagation
+      std::vector<ComponentCache>& componentCache = result.componentCache;
+      componentCache.clear();
+
       convoluteComponents(state, stepper, navigator, tmpStates, componentCache);
 
       if (componentCache.empty()) {