From 4cd48be6d30e5c74e6740b84003c1c21b92a7e3a Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Tue, 9 Jun 2020 17:40:10 +0200 Subject: [PATCH 01/19] added reset of TrackStateProvider cache to TrackMasterFitter --- Tr/TrackFitter/src/TrackMasterFitter.cpp | 3 +++ Tr/TrackFitter/src/TrackMasterFitter.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/Tr/TrackFitter/src/TrackMasterFitter.cpp b/Tr/TrackFitter/src/TrackMasterFitter.cpp index b439db15ead..de266d2cf26 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.cpp +++ b/Tr/TrackFitter/src/TrackMasterFitter.cpp @@ -263,6 +263,9 @@ StatusCode TrackMasterFitter::fit_r( Track& track, std::any& accelCache, LHCb::P // any track that doesnt make it to the end is failed track.setFitStatus( Track::FitStatus::FitFailed ); + // make sure to reset the stateprovider cache, no matter what happens next. + const_cast<ITrackStateProvider&>(*m_stateProvider).clearCache( track ) ; + // create the KalmanFitResult if it doesn't exist yet LHCb::KalmanFitResult* kalfitresult = dynamic_cast<LHCb::KalmanFitResult*>( track.fitResult() ); if ( !kalfitresult ) { diff --git a/Tr/TrackFitter/src/TrackMasterFitter.h b/Tr/TrackFitter/src/TrackMasterFitter.h index 616cf30c6f6..88f384d45b7 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.h +++ b/Tr/TrackFitter/src/TrackMasterFitter.h @@ -24,6 +24,7 @@ #include "TrackInterfaces/ITrackFitter.h" #include "TrackInterfaces/ITrackKalmanFilter.h" #include "TrackInterfaces/ITrackProjectorSelector.h" +#include "TrackInterfaces/ITrackStateProvider.h" // Forward declarations struct ITrackManipulator; @@ -114,6 +115,7 @@ private: ToolHandle<IMeasurementProvider> m_measProvider; ToolHandle<IMaterialLocator> m_materialLocator; ToolHandle<ITrackProjectorSelector> m_projectorSelector; + PublicToolHandle<ITrackStateProvider> m_stateProvider{this,"StateProvider","TrackStateProvider"} ; private: // job options -- GitLab From 09f32a84748da6fc2bb4696f1bf1ebb46926bdbe Mon Sep 17 00:00:00 2001 From: Gitlab CI <noreply@cern.ch> Date: Tue, 9 Jun 2020 15:41:37 +0000 Subject: [PATCH 02/19] Fixed formatting patch generated by https://gitlab.cern.ch/lhcb/Rec/-/jobs/8729478 --- Tr/TrackFitter/src/TrackMasterFitter.cpp | 2 +- Tr/TrackFitter/src/TrackMasterFitter.h | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Tr/TrackFitter/src/TrackMasterFitter.cpp b/Tr/TrackFitter/src/TrackMasterFitter.cpp index de266d2cf26..c570044ec0a 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.cpp +++ b/Tr/TrackFitter/src/TrackMasterFitter.cpp @@ -264,7 +264,7 @@ StatusCode TrackMasterFitter::fit_r( Track& track, std::any& accelCache, LHCb::P track.setFitStatus( Track::FitStatus::FitFailed ); // make sure to reset the stateprovider cache, no matter what happens next. - const_cast<ITrackStateProvider&>(*m_stateProvider).clearCache( track ) ; + const_cast<ITrackStateProvider&>( *m_stateProvider ).clearCache( track ); // create the KalmanFitResult if it doesn't exist yet LHCb::KalmanFitResult* kalfitresult = dynamic_cast<LHCb::KalmanFitResult*>( track.fitResult() ); diff --git a/Tr/TrackFitter/src/TrackMasterFitter.h b/Tr/TrackFitter/src/TrackMasterFitter.h index 88f384d45b7..b77a3c316a6 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.h +++ b/Tr/TrackFitter/src/TrackMasterFitter.h @@ -109,13 +109,13 @@ private: } private: - ToolHandle<ITrackExtrapolator> m_extrapolator; ///< extrapolator - ToolHandle<ITrackExtrapolator> m_veloExtrapolator; ///< extrapolator for Velo-only tracks - ToolHandle<ITrackKalmanFilter> m_trackNodeFitter; ///< delegate to actual track fitter (which fits from nodes) - ToolHandle<IMeasurementProvider> m_measProvider; - ToolHandle<IMaterialLocator> m_materialLocator; - ToolHandle<ITrackProjectorSelector> m_projectorSelector; - PublicToolHandle<ITrackStateProvider> m_stateProvider{this,"StateProvider","TrackStateProvider"} ; + ToolHandle<ITrackExtrapolator> m_extrapolator; ///< extrapolator + ToolHandle<ITrackExtrapolator> m_veloExtrapolator; ///< extrapolator for Velo-only tracks + ToolHandle<ITrackKalmanFilter> m_trackNodeFitter; ///< delegate to actual track fitter (which fits from nodes) + ToolHandle<IMeasurementProvider> m_measProvider; + ToolHandle<IMaterialLocator> m_materialLocator; + ToolHandle<ITrackProjectorSelector> m_projectorSelector; + PublicToolHandle<ITrackStateProvider> m_stateProvider{this, "StateProvider", "TrackStateProvider"}; private: // job options -- GitLab From a45141126fa2d5ad2bbea6fdf1d9eb99abdf81e1 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Fri, 12 Jun 2020 11:58:53 +0200 Subject: [PATCH 03/19] made access to cache const --- Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h b/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h index 752d2c526e4..59bca3ab338 100644 --- a/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h +++ b/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h @@ -53,9 +53,9 @@ struct ITrackStateProvider : extend_interfaces<IAlgTool> { virtual const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const = 0; /// Clear the cache - virtual void clearCache() = 0; + virtual void clearCache() const = 0; /// Clear the cache for a particular track - virtual void clearCache( const LHCb::Track& track ) = 0; + virtual void clearCache( const LHCb::Track& track ) const = 0; }; #endif // TRACKINTERFACES_ITRACKSTATEPROVIDER_H -- GitLab From a1aa19482883d7896b3484ac34c9563d40e924b5 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Fri, 12 Jun 2020 12:02:48 +0200 Subject: [PATCH 04/19] removed const_cast --- Tr/TrackFitter/src/TrackMasterFitter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tr/TrackFitter/src/TrackMasterFitter.cpp b/Tr/TrackFitter/src/TrackMasterFitter.cpp index c570044ec0a..6c1b14d0e62 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.cpp +++ b/Tr/TrackFitter/src/TrackMasterFitter.cpp @@ -264,7 +264,7 @@ StatusCode TrackMasterFitter::fit_r( Track& track, std::any& accelCache, LHCb::P track.setFitStatus( Track::FitStatus::FitFailed ); // make sure to reset the stateprovider cache, no matter what happens next. - const_cast<ITrackStateProvider&>( *m_stateProvider ).clearCache( track ); + m_stateProvider->clearCache( track ); // create the KalmanFitResult if it doesn't exist yet LHCb::KalmanFitResult* kalfitresult = dynamic_cast<LHCb::KalmanFitResult*>( track.fitResult() ); -- GitLab From da78e29e5e16cc5523ed0db15d2b3e96bc680020 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Fri, 12 Jun 2020 13:07:36 +0200 Subject: [PATCH 05/19] made the trackid based on hash; removed check that track is in TES; added mutex locks to try and make code thread safe --- .../src/TrackStateProvider.cpp | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index a15d2edcf9f..ebae06bcde1 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -88,17 +88,17 @@ public: const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override; /// Clear the cache - void clearCache() override; + void clearCache() const override; /// Clear the cache for a particular track - void clearCache( const LHCb::Track& track ) override; + void clearCache( const LHCb::Track& track ) const override; /// incident service handle void handle( const Incident& incident ) override; private: /// Type for cache key - using TkCacheKey = std::uintptr_t; + using TkCacheKey = std::size_t; StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; @@ -115,15 +115,16 @@ private: /// Create the key for a given track inline TkCacheKey trackID( const LHCb::Track& track ) const { - // warn if operating on a track with no parent container. - if ( !track.parent() ) { - Error( "Track has no parent container. TrackStateProvider cache might be unreliable.." ).ignore(); - } // key is based on track memory addresses - return TkCacheKey( &track ); + TkCacheKey id = TkCacheKey(&track) ; + boost::hash_combine(id, track.fitResult() ) ; + boost::hash_combine(id, track.lhcbIDs().front().lhcbID() ) ; + boost::hash_combine(id, track.lhcbIDs().back().lhcbID() ) ; + return id ; } private: + mutable std::mutex m_cachelock ; mutable std::unordered_map<TkCacheKey, std::unique_ptr<::TrackCache>> m_trackcache; ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; @@ -261,25 +262,29 @@ void TrackStateProvider::handle( const Incident& /* incident */ ) { // Clear cache (and update some counters) //============================================================================= -void TrackStateProvider::clearCache() { +void TrackStateProvider::clearCache() const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; counter( "Number of tracks seen" ) += m_trackcache.size(); if ( !m_trackcache.empty() ) { + std::lock_guard lockguard( m_cachelock ) ; counter( "Number of states added" ) += std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, []( auto n, const auto& p ) { return n + p.second->numOwnedStates(); } ); + m_trackcache.clear(); } - m_trackcache.clear(); } //============================================================================= // Clear cache for a given track //============================================================================= -void TrackStateProvider::clearCache( const LHCb::Track& track ) { +void TrackStateProvider::clearCache( const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; auto it = m_trackcache.find( trackID( track ) ); - if ( it != m_trackcache.end() ) m_trackcache.erase( it ); + if ( it != m_trackcache.end() ) { + std::lock_guard lockguard( m_cachelock ) ; + m_trackcache.erase( it ); + } } //============================================================================= @@ -342,6 +347,7 @@ TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location lo if ( msgLevel( MSG::DEBUG ) ) { debug() << "Adding state to track cache: key=" << tc.track().key() << " " << z << " " << loc << endmsg; } + std::lock_guard lockguard( m_cachelock ) ; return tc.insertState( it, std::move( state ) ); } @@ -444,6 +450,7 @@ TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { auto key = trackID( track ); auto it = m_trackcache.find( key ); if ( it == m_trackcache.end() ) { + std::lock_guard lockguard( m_cachelock ) ; auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); it = ret.first; } -- GitLab From a9fe9f73a5cbfb45c1e3c549d7163cf812780c1f Mon Sep 17 00:00:00 2001 From: Gitlab CI <noreply@cern.ch> Date: Fri, 12 Jun 2020 11:09:54 +0000 Subject: [PATCH 06/19] Fixed formatting patch generated by https://gitlab.cern.ch/lhcb/Rec/-/jobs/8778419 --- .../src/TrackStateProvider.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index ebae06bcde1..f4648b4bb86 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -116,15 +116,15 @@ private: /// Create the key for a given track inline TkCacheKey trackID( const LHCb::Track& track ) const { // key is based on track memory addresses - TkCacheKey id = TkCacheKey(&track) ; - boost::hash_combine(id, track.fitResult() ) ; - boost::hash_combine(id, track.lhcbIDs().front().lhcbID() ) ; - boost::hash_combine(id, track.lhcbIDs().back().lhcbID() ) ; - return id ; + TkCacheKey id = TkCacheKey( &track ); + boost::hash_combine( id, track.fitResult() ); + boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); + boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); + return id; } private: - mutable std::mutex m_cachelock ; + mutable std::mutex m_cachelock; mutable std::unordered_map<TkCacheKey, std::unique_ptr<::TrackCache>> m_trackcache; ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; @@ -266,7 +266,7 @@ void TrackStateProvider::clearCache() const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; counter( "Number of tracks seen" ) += m_trackcache.size(); if ( !m_trackcache.empty() ) { - std::lock_guard lockguard( m_cachelock ) ; + std::lock_guard lockguard( m_cachelock ); counter( "Number of states added" ) += std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, []( auto n, const auto& p ) { return n + p.second->numOwnedStates(); } ); @@ -282,7 +282,7 @@ void TrackStateProvider::clearCache( const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; auto it = m_trackcache.find( trackID( track ) ); if ( it != m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ) ; + std::lock_guard lockguard( m_cachelock ); m_trackcache.erase( it ); } } @@ -347,7 +347,7 @@ TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location lo if ( msgLevel( MSG::DEBUG ) ) { debug() << "Adding state to track cache: key=" << tc.track().key() << " " << z << " " << loc << endmsg; } - std::lock_guard lockguard( m_cachelock ) ; + std::lock_guard lockguard( m_cachelock ); return tc.insertState( it, std::move( state ) ); } @@ -450,9 +450,9 @@ TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { auto key = trackID( track ); auto it = m_trackcache.find( key ); if ( it == m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ) ; - auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); - it = ret.first; + std::lock_guard lockguard( m_cachelock ); + auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); + it = ret.first; } return *it->second; } -- GitLab From 788490706261e1d18b1d9cd116ed92c7fb531436 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Mon, 15 Jun 2020 08:34:11 +0200 Subject: [PATCH 07/19] Make trackstateprovider use TES --- .../src/TrackStateProvider.cpp | 236 +++++++++--------- 1 file changed, 124 insertions(+), 112 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index f4648b4bb86..73dd4ffb684 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -15,6 +15,8 @@ #include "GaudiKernel/IIncidentListener.h" #include "GaudiKernel/IIncidentSvc.h" #include "GaudiKernel/ToolHandle.h" +#include "GaudiKernel/DataObjectHandle.h" + // from TrackInterfaces #include "TrackInterfaces/ITrackExtrapolator.h" @@ -51,98 +53,10 @@ * @date 14/08/2010 **/ -// forward declarations -namespace { - class TrackCache; -} - -class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider, IIncidentListener> { - -public: - /// Standard constructor - TrackStateProvider( const std::string& type, const std::string& name, const IInterface* parent ); - - /// initialize - StatusCode initialize() override; - - /// initialize - StatusCode finalize() override; - - /// Compute the state of the track at position z. The third - /// argument is the tolerance: if an existing state is found within - /// a z-distance 'tolerance', that state is returned. - /// If there are 'fitnodes' on the track (e.g. in Brunel), this - /// method will use interpolation. If there are no fitnodes (e.g. in - /// DaVinci) the method will use extrapolation. In that case the - /// answer is only correct outside the measurement range. - StatusCode state( LHCb::State& astate, const LHCb::Track& track, double z, double ztolerance ) const override; - - /// Compute state using cached trajectory - StatusCode stateFromTrajectory( LHCb::State& state, const LHCb::Track& track, double z ) const override { - const auto traj = trajectory( track ); - if ( traj ) { state = traj->state( z ); } - return ( traj ? StatusCode::SUCCESS : StatusCode::FAILURE ); - } - - /// Retrieve the cached trajectory - const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override; - - /// Clear the cache - void clearCache() const override; - - /// Clear the cache for a particular track - void clearCache( const LHCb::Track& track ) const override; - - /// incident service handle - void handle( const Incident& incident ) override; - -private: - /// Type for cache key - using TkCacheKey = std::size_t; - - StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, - const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; - - const LHCb::State* - addState( TrackCache& tc, double z, LHCb::State::Location loc = LHCb::State::LocationUnknown, - boost::optional<LHCb::TrackTraj::StateContainer::const_iterator> position = boost::none ) const; - - /// Create a cache entry - std::unique_ptr<TrackCache> createCacheEntry( const TkCacheKey key, const LHCb::Track& track ) const; - - /// Retrieve a cache entry - TrackCache& cache( const LHCb::Track& track ) const; - - /// Create the key for a given track - inline TkCacheKey trackID( const LHCb::Track& track ) const { - // key is based on track memory addresses - TkCacheKey id = TkCacheKey( &track ); - boost::hash_combine( id, track.fitResult() ); - boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); - boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); - return id; - } - -private: - mutable std::mutex m_cachelock; - mutable std::unordered_map<TkCacheKey, std::unique_ptr<::TrackCache>> m_trackcache; - ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; - ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; - - Gaudi::Property<bool> m_applyMaterialCorrections{this, "ApplyMaterialCorrections", true}; - Gaudi::Property<double> m_linearPropagationTolerance{this, "LinearPropagationTolerance", 1.0 * Gaudi::Units::mm}; - Gaudi::Property<bool> m_cacheStatesOnDemand{this, "CacheStatesOnDemand", false}; -}; - -/**********************************************************************************************/ +// forward declarations namespace { - - constexpr struct compareStateZ_t { - bool operator()( const LHCb::State* lhs, const LHCb::State* rhs ) const { return lhs->z() < rhs->z(); } - } compareStateZ{}; - - // The TrackCache is basically just an 'extendable' TrackTraj + // The TrackCache is basically just an 'extendable' TrackTraj class TrackCache final : public LHCb::TrackTraj { private: @@ -175,6 +89,11 @@ namespace { std::size_t numOwnedStates() const noexcept { return m_ownedstates.size(); } }; + + constexpr struct compareStateZ_t { + bool operator()( const LHCb::State* lhs, const LHCb::State* rhs ) const { return lhs->z() < rhs->z(); } + } compareStateZ{}; + /// Add a state const LHCb::State* TrackCache::insertState( StateContainer::const_iterator pos, std::unique_ptr<LHCb::State> o_state ) { @@ -212,7 +131,104 @@ namespace { return state; } -} // namespace + + /// Type for cache key + using TkCacheKey = std::size_t; + using TrackCaches = std::unordered_map<TkCacheKey,TrackCache> ; + using ThreadTrackCaches = std::unordered_map<std::thread::id,TrackCaches> ; + + /// Template specialization + // template <> + // AnyDataWrapper<TrackCaches> DataObjectHandle<AnyDataWrapper<TrackCaches>>::getOrCreate() const { + // auto* obj = get(false); + // return obj ? obj : put( std::make_unique< AnyDataWrapper<TrackCaches> >( TrackCaches{} ) ); + // } +} //namespace + +class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider, IIncidentListener> { + +public: + /// Standard constructor + TrackStateProvider( const std::string& type, const std::string& name, const IInterface* parent ); + + /// initialize + StatusCode initialize() override; + + /// initialize + StatusCode finalize() override; + + /// Compute the state of the track at position z. The third + /// argument is the tolerance: if an existing state is found within + /// a z-distance 'tolerance', that state is returned. + /// If there are 'fitnodes' on the track (e.g. in Brunel), this + /// method will use interpolation. If there are no fitnodes (e.g. in + /// DaVinci) the method will use extrapolation. In that case the + /// answer is only correct outside the measurement range. + StatusCode state( LHCb::State& astate, const LHCb::Track& track, double z, double ztolerance ) const override; + + /// Compute state using cached trajectory + StatusCode stateFromTrajectory( LHCb::State& state, const LHCb::Track& track, double z ) const override { + const auto traj = trajectory( track ); + if ( traj ) { state = traj->state( z ); } + return ( traj ? StatusCode::SUCCESS : StatusCode::FAILURE ); + } + + /// Retrieve the cached trajectory + const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override; + + /// Clear the cache + void clearCache() const override; + + /// Clear the cache for a particular track + void clearCache( const LHCb::Track& track ) const override; + + /// incident service handle + void handle( const Incident& incident ) override; + +private: + + StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, + const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; + + const LHCb::State* + addState( TrackCache& tc, double z, LHCb::State::Location loc = LHCb::State::LocationUnknown, + boost::optional<LHCb::TrackTraj::StateContainer::const_iterator> position = boost::none ) const; + + /// Get the track cache from the event store + TrackCaches& trackcache() const { + auto* obj = m_caches.getIfExists(); + return const_cast<TrackCaches&>(obj ? *obj : *(m_caches.put( TrackCaches{} ))) ; + } + + /// Create a cache entry + TrackCache createCacheEntry( const TkCacheKey key, const LHCb::Track& track ) const; + + /// Retrieve a single cache entry + TrackCache& cache( const LHCb::Track& track ) const; + + /// Create the key for a given track + inline TkCacheKey trackID( const LHCb::Track& track ) const { + // key is based on track memory addresses + TkCacheKey id = TkCacheKey( &track ); + boost::hash_combine( id, track.fitResult() ); + if( !track.lhcbIDs().empty() ) { + boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); + boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); + } + return id; + } + +private: + mutable DataObjectHandle<AnyDataWrapper<TrackCaches>> m_caches { this, Gaudi::DataHandle::Writer, "CacheLocation", "TrackStateProviderCache" }; + ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; + ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; + + Gaudi::Property<bool> m_applyMaterialCorrections{this, "ApplyMaterialCorrections", true}; + Gaudi::Property<double> m_linearPropagationTolerance{this, "LinearPropagationTolerance", 1.0 * Gaudi::Units::mm}; + Gaudi::Property<bool> m_cacheStatesOnDemand{this, "CacheStatesOnDemand", false}; +}; + +/**********************************************************************************************/ DECLARE_COMPONENT( TrackStateProvider ) @@ -234,7 +250,7 @@ StatusCode TrackStateProvider::initialize() { if ( sc.isSuccess() ) sc = m_extrapolator.retrieve(); if ( sc.isSuccess() ) sc = m_interpolator.retrieve(); // reset at the start of each event - incSvc()->addListener( this, IncidentType::BeginEvent ); + //incSvc()->addListener( this, IncidentType::BeginEvent ); return sc; } @@ -242,7 +258,7 @@ StatusCode TrackStateProvider::initialize() { // Finalize //============================================================================= StatusCode TrackStateProvider::finalize() { - clearCache(); + //clearCache(); m_extrapolator.release().ignore(); m_interpolator.release().ignore(); return GaudiTool::finalize(); @@ -263,15 +279,13 @@ void TrackStateProvider::handle( const Incident& /* incident */ ) { //============================================================================= void TrackStateProvider::clearCache() const { + auto& m_trackcache = trackcache() ; if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; counter( "Number of tracks seen" ) += m_trackcache.size(); - if ( !m_trackcache.empty() ) { - std::lock_guard lockguard( m_cachelock ); - counter( "Number of states added" ) += - std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, - []( auto n, const auto& p ) { return n + p.second->numOwnedStates(); } ); - m_trackcache.clear(); - } + counter( "Number of states added" ) += + std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, + []( auto n, const auto& p ) { return n + p.second.numOwnedStates(); } ); + m_trackcache.clear(); } //============================================================================= @@ -280,11 +294,9 @@ void TrackStateProvider::clearCache() const { void TrackStateProvider::clearCache( const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; + auto& m_trackcache = trackcache() ; auto it = m_trackcache.find( trackID( track ) ); - if ( it != m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ); - m_trackcache.erase( it ); - } + if ( it != m_trackcache.end() ) m_trackcache.erase( it ); } //============================================================================= @@ -347,7 +359,6 @@ TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location lo if ( msgLevel( MSG::DEBUG ) ) { debug() << "Adding state to track cache: key=" << tc.track().key() << " " << z << " " << loc << endmsg; } - std::lock_guard lockguard( m_cachelock ); return tc.insertState( it, std::move( state ) ); } @@ -394,20 +405,20 @@ StatusCode TrackStateProvider::computeState( const TrackCache& tc, double z, LHC // retrieve the cache for a given track //============================================================================= -std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Track& track ) const { +TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Creating track cache for track: key=" << track.key() << " hashID=" << key << " " << track.states().size() << endmsg; // create a new entry in the cache - auto tc = std::make_unique<TrackCache>( track ); + TrackCache tc{track} ; // make sure downstream tracks have a few ref states before the first measurement. if ( track.type() == LHCb::Track::Downstream ) { for ( const auto& loc : {std::make_pair( LHCb::State::EndRich1, StateParameters::ZEndRich1 ), std::make_pair( LHCb::State::BegRich1, StateParameters::ZBegRich1 ), std::make_pair( LHCb::State::EndVelo, StateParameters::ZEndVelo )} ) { - if ( !track.stateAt( loc.first ) ) { addState( *tc, loc.second, loc.first ); } + if ( !track.stateAt( loc.first ) ) { addState( tc, loc.second, loc.first ); } } } @@ -425,7 +436,7 @@ std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key if ( dz < -10 * Gaudi::Units::cm ) { auto z = track.firstState().z() + dz; if ( z > -100 * Gaudi::Units::cm ) { // beginning of velo - addState( *tc, z, LHCb::State::ClosestToBeam ); + addState( tc, z, LHCb::State::ClosestToBeam ); } } } @@ -437,7 +448,7 @@ std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key // the end of the Velo. if ( !track.checkFlag( LHCb::Track::Backward ) && track.hasVelo() && !track.stateAt( LHCb::State::FirstMeasurement ) ) { - addState( *tc, StateParameters::ZEndVelo, LHCb::State::EndVelo ); + addState( tc, StateParameters::ZEndVelo, LHCb::State::EndVelo ); } return tc; @@ -447,14 +458,15 @@ std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key // retrieve TrackCache entry for a given track (from the cache) //============================================================================= TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { + // get the cache from the stack auto key = trackID( track ); + auto& m_trackcache = trackcache() ; auto it = m_trackcache.find( key ); if ( it == m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ); auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); it = ret.first; } - return *it->second; + return it->second; } const LHCb::TrackTraj* TrackStateProvider::trajectory( const LHCb::Track& track ) const { return &cache( track ); } -- GitLab From 47f7feda2d4fd830010a7d15d42480da9e5a3164 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Mon, 15 Jun 2020 13:46:15 +0200 Subject: [PATCH 08/19] - quasi fancy speedup: use a thread dependent pointer to eventstore objects - some code modernization --- .../src/TrackStateProvider.cpp | 316 +++++++++++------- 1 file changed, 187 insertions(+), 129 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index 73dd4ffb684..94ba82cfb9e 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -1,5 +1,5 @@ /*****************************************************************************\ -* (c) Copyright 2000-2019 CERN for the benefit of the LHCb Collaboration * +* (c) Copyright 2000-2020 CERN for the benefit of the LHCb Collaboration * * * * This software is distributed under the terms of the GNU General Public * * Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * @@ -8,16 +8,12 @@ * granted to it by virtue of its status as an Intergovernmental Organization * * or submit itself to any jurisdiction. * \*****************************************************************************/ -// Include files -// ------------- + // from Gaudi #include "GaudiAlg/GaudiTool.h" -#include "GaudiKernel/IIncidentListener.h" -#include "GaudiKernel/IIncidentSvc.h" #include "GaudiKernel/ToolHandle.h" #include "GaudiKernel/DataObjectHandle.h" - // from TrackInterfaces #include "TrackInterfaces/ITrackExtrapolator.h" #include "TrackInterfaces/ITrackInterpolator.h" @@ -34,15 +30,16 @@ #include "LHCbMath/Similarity.h" #include "TrackKernel/TrackTraj.h" -// boost -#include "boost/optional.hpp" -//#include "boost/functional/hash.hpp" - // STL #include <cstdint> #include <memory> +#include <mutex> #include <numeric> #include <unordered_map> +#include <thread> + +// boost +#include "boost/functional/hash.hpp" /** @class TrackStateProvider TrackStateProvider.h * @@ -53,20 +50,39 @@ * @date 14/08/2010 **/ - -// forward declarations namespace { - // The TrackCache is basically just an 'extendable' TrackTraj + + /// Type for cache key + using TkCacheKey = std::uintptr_t; + + /// compare states by Z position + inline constexpr auto compareStateZ = []( const LHCb::State* lhs, const LHCb::State* rhs ) { + return lhs->z() < rhs->z(); + }; + + /// Create the key for a given track + inline TkCacheKey trackID( const LHCb::Track& track ) { + // key is based on track memory addresses + TkCacheKey id = TkCacheKey( &track ); + boost::hash_combine( id, track.fitResult() ); + if( !track.lhcbIDs().empty() ) { + boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); + boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); + } + return id; + } + + // The TrackCache is basically just an 'extendable' TrackTraj class TrackCache final : public LHCb::TrackTraj { private: - const LHCb::Track* m_track = nullptr; - std::vector<std::unique_ptr<LHCb::State>> m_ownedstates; - double m_zFirstMeasurement{-9999}; + const LHCb::Track* m_track{nullptr}; + std::deque<LHCb::State> m_ownedstates; + double m_zFirstMeasurement{-9999}; public: TrackCache( const LHCb::Track& track ) : LHCb::TrackTraj( track ), m_track( &track ) { - const auto state = track.stateAt( LHCb::State::FirstMeasurement ); + const auto state = track.stateAt( LHCb::State::Location::FirstMeasurement ); m_zFirstMeasurement = ( state ? state->z() : -9999 ); } @@ -76,52 +92,30 @@ namespace { /// return z position of state at first measurement double zFirstMeasurement() const noexcept { return m_zFirstMeasurement; } - // insert a new state in the container with states - const LHCb::State* insertState( StateContainer::const_iterator pos, std::unique_ptr<LHCb::State> state ); + /// insert a new state in the container with states + const LHCb::State* insertState( std::ptrdiff_t pos, LHCb::State&& state ); - // return the states (work around keyword protected) - StateContainer& states() noexcept { return refStates(); } + /// return the states (work around keyword protected) + auto& states() noexcept { return refStates(); } - // return the states (work around keyword protected) - const StateContainer& states() const noexcept { return refStates(); } + /// return the states (work around keyword protected) + const auto& states() const noexcept { return refStates(); } - // return number of owned states + /// return number of owned states std::size_t numOwnedStates() const noexcept { return m_ownedstates.size(); } }; - - constexpr struct compareStateZ_t { - bool operator()( const LHCb::State* lhs, const LHCb::State* rhs ) const { return lhs->z() < rhs->z(); } - } compareStateZ{}; - - /// Add a state - const LHCb::State* TrackCache::insertState( StateContainer::const_iterator pos, - std::unique_ptr<LHCb::State> o_state ) { - + const LHCb::State* TrackCache::insertState( std::ptrdiff_t pos, LHCb::State&& o_state ) { + // take ownership of state - m_ownedstates.push_back( std::move( o_state ) ); - auto state = m_ownedstates.back().get(); + m_ownedstates.push_back( o_state ) ; + auto state = &(m_ownedstates.back()) ; // get the vector with states - StateContainer& refstates = refStates(); - - // temporary logic test - // if( refstates.size()>0) { - // if(pos == refstates.end() ) { - // assert( refstates.back()->z() < state->z() ) ; - // } else if ( pos == refstates.begin() ) { - // assert( refstates.front()->z() > state->z() ) ; - // } else { - // StateContainer::iterator prev = pos ; --prev ; - // assert( (*prev)->z() < state->z() && state->z() < (*pos)->z() ) ; - // } - // } - - // insert the state. gcc 4.8 has trouble with the - // const_iterator. let's hope that the optimization does the - // obvious with the following strange cast. - auto nonconstpos = refstates.begin() + std::distance( refstates.cbegin(), pos ); - refstates.insert( nonconstpos, state ); + auto& refstates = refStates(); + + refstates.insert( std::next( refstates.begin(), pos ), state ); + assert( std::is_sorted( refstates.begin(), refstates.end(), compareStateZ ) ); // update the range of the trajectory Trajectory::setRange( refstates.front()->z(), refstates.back()->z() ); @@ -132,20 +126,83 @@ namespace { return state; } - /// Type for cache key - using TkCacheKey = std::size_t; + /// Type for container of track caches using TrackCaches = std::unordered_map<TkCacheKey,TrackCache> ; - using ThreadTrackCaches = std::unordered_map<std::thread::id,TrackCaches> ; + + // Need to solve two problems at once + // * there can be multiple tool instances sharing the same cache + // * there can be multiple threads accessing different caches in the event store + // I'd like all that logic to be inside the handle + + // wrapper class to hold object with 'subscribed' pointers + template<typename T> + class SubscribedObject : public T + { + public: + // need to implement a move constructor and forbid a copy constructor + //using T::T ; + SubscribedObject() = default ; + SubscribedObject( const SubscribedObject& ) = delete ; + SubscribedObject( SubscribedObject&& rhs ) : T{rhs}, m_subscribers{rhs.m_subscribers} { + for( auto& p : m_subscribers ) *p = this ; + } + ~SubscribedObject() { + for( auto& p : m_subscribers ) *p = nullptr ; + } + void addsubscriber( T*& p) { + m_subscribers.push_back( &p ) ; + p = this ; + } + private: + std::vector< T** > m_subscribers ; + } ; + + // wrapper to hold Subscribed objects with fast pointers that depend on the thread + template<typename T> + class ThreadedAnyDataObjectSmartHandle : public DataObjectHandle<AnyDataWrapper<SubscribedObject<T> > > + { + public: + using BASE = DataObjectHandle<AnyDataWrapper<SubscribedObject<T> > > ; + //using BASE::BASE ; + + /// Autodeclaring constructor with property name, mode, key and documentation. + /// @note the use std::enable_if is required to avoid ambiguities + template <class OWNER, class K, typename = std::enable_if_t<std::is_base_of_v<IProperty, OWNER>>> + ThreadedAnyDataObjectSmartHandle( OWNER* owner, Gaudi::DataHandle::Mode m, std::string name, const K& key = {}, + std::string doc = "" ) + : BASE{ key, m, owner } { + auto p = owner->declareProperty( std::move( name ), *this, std::move( doc ) ); + p->template setOwnerType<OWNER>(); + } - /// Template specialization - // template <> - // AnyDataWrapper<TrackCaches> DataObjectHandle<AnyDataWrapper<TrackCaches>>::getOrCreate() const { - // auto* obj = get(false); - // return obj ? obj : put( std::make_unique< AnyDataWrapper<TrackCaches> >( TrackCaches{} ) ); - // } + T* get( std::mutex& mutex) const { + // add an entry to the map, if it doesn't exist yet. this should only happen once per thread. + const auto threadid = std::this_thread::get_id() ; + auto it = m_threadcachemap.find(threadid) ; + if( it == m_threadcachemap.end() ) { + std::scoped_lock lock{ mutex } ; + auto insertion = m_threadcachemap.insert( std::pair{threadid, nullptr} ) ; + it = insertion.first ; + } + if( !it->second ) { + // first check if it is not already in the event store + auto* obj = this->getIfExists() ; + // if not, add it. this should happen only once per event per thread. + if( !obj ) { + obj = const_cast<SubscribedObject<T>*>(this->put(SubscribedObject<T>{})); + } + obj->addsubscriber( it->second ) ; + } + return const_cast<T*>(it->second) ; + } + + private: + mutable std::unordered_map< std::thread::id, T* > m_threadcachemap ; + } ; + } //namespace -class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider, IIncidentListener> { +class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider> { public: /// Standard constructor @@ -174,7 +231,7 @@ public: } /// Retrieve the cached trajectory - const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override; + const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override { return &cache( track ); } /// Clear the cache void clearCache() const override; @@ -182,22 +239,23 @@ public: /// Clear the cache for a particular track void clearCache( const LHCb::Track& track ) const override; - /// incident service handle - void handle( const Incident& incident ) override; - private: - StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, - const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; - - const LHCb::State* - addState( TrackCache& tc, double z, LHCb::State::Location loc = LHCb::State::LocationUnknown, - boost::optional<LHCb::TrackTraj::StateContainer::const_iterator> position = boost::none ) const; + StatusCode computeState( const TrackCache& tc, // + const double z, // + LHCb::State& state, // + std::ptrdiff_t position ) const; + + const LHCb::State* addState( TrackCache& tc, // + double z, // + LHCb::State::Location loc = LHCb::State::Location::LocationUnknown, + std::ptrdiff_t position = -1 ) const; /// Get the track cache from the event store TrackCaches& trackcache() const { - auto* obj = m_caches.getIfExists(); - return const_cast<TrackCaches&>(obj ? *obj : *(m_caches.put( TrackCaches{} ))) ; + return const_cast<TrackCaches&>(*m_caches.get(m_mutex) ) ; + //auto* obj = m_caches.getIfExists(); + //return const_cast<TrackCaches&>(obj ? *obj : *(m_caches.put( TrackCaches{} ))) ; } /// Create a cache entry @@ -219,7 +277,8 @@ private: } private: - mutable DataObjectHandle<AnyDataWrapper<TrackCaches>> m_caches { this, Gaudi::DataHandle::Writer, "CacheLocation", "TrackStateProviderCache" }; + mutable std::mutex m_mutex ; + mutable ThreadedAnyDataObjectSmartHandle<TrackCaches> m_caches { this, Gaudi::DataHandle::Writer, "CacheLocation", "TrackStateProviderCache" }; ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; @@ -235,7 +294,9 @@ DECLARE_COMPONENT( TrackStateProvider ) //============================================================================= // TrackStateProvider constructor. //============================================================================= -TrackStateProvider::TrackStateProvider( const std::string& type, const std::string& name, const IInterface* parent ) +TrackStateProvider::TrackStateProvider( const std::string& type, // + const std::string& name, // + const IInterface* parent ) : base_class( type, name, parent ) { declareProperty( "Extrapolator", m_extrapolator ); declareProperty( "Interpolator", m_interpolator ); @@ -249,8 +310,6 @@ StatusCode TrackStateProvider::initialize() { // retrieve tools if ( sc.isSuccess() ) sc = m_extrapolator.retrieve(); if ( sc.isSuccess() ) sc = m_interpolator.retrieve(); - // reset at the start of each event - //incSvc()->addListener( this, IncidentType::BeginEvent ); return sc; } @@ -258,22 +317,11 @@ StatusCode TrackStateProvider::initialize() { // Finalize //============================================================================= StatusCode TrackStateProvider::finalize() { - //clearCache(); m_extrapolator.release().ignore(); m_interpolator.release().ignore(); return GaudiTool::finalize(); } -//============================================================================= -// Incident handle -//============================================================================= - -void TrackStateProvider::handle( const Incident& /* incident */ ) { - // only one incident type subscribed to, so no need to check type - // if ( IncidentType::BeginEvent == incident.type() ) - clearCache(); -} - //============================================================================= // Clear cache (and update some counters) //============================================================================= @@ -303,8 +351,10 @@ void TrackStateProvider::clearCache( const LHCb::Track& track ) const { // get a state at a particular z position, within given tolerance //============================================================================= -StatusCode TrackStateProvider::state( LHCb::State& state, const LHCb::Track& track, double z, - double ztolerance ) const { +StatusCode TrackStateProvider::state( LHCb::State& state, // + const LHCb::Track& track, // + double z, // + double ztolerance ) const { auto& tc = cache( track ); // locate the closest state with lower_bound, but cache the @@ -326,8 +376,11 @@ StatusCode TrackStateProvider::state( LHCb::State& state, const LHCb::Track& tra const auto absdz = std::abs( z - closeststate->z() ); if ( absdz > std::max( ztolerance, TrackParameters::propagationTolerance ) ) { if ( absdz > m_linearPropagationTolerance ) { - if ( !m_cacheStatesOnDemand ) { return computeState( tc, z, state, position ); } - const auto newstate = addState( tc, z, LHCb::State::LocationUnknown, position ); + if ( !m_cacheStatesOnDemand ) { + return computeState( tc, z, state, std::distance( tc.states().cbegin(), position ) ); + } + const auto newstate = + addState( tc, z, LHCb::State::Location::LocationUnknown, std::distance( tc.states().cbegin(), position ) ); counter( "AddedNumberOnDemandStates" ) += 1; if ( !newstate ) return StatusCode::FAILURE; state = *newstate; @@ -346,42 +399,47 @@ StatusCode TrackStateProvider::state( LHCb::State& state, const LHCb::Track& tra // add a state to the cache of a given track //============================================================================= -const LHCb::State* -TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location loc, - boost::optional<LHCb::TrackTraj::StateContainer::const_iterator> position ) const { - auto state = std::make_unique<LHCb::State>( loc ); - state->setZ( z ); - auto it = ( position ? *position - : std::lower_bound( tc.states().cbegin(), tc.states().cend(), state.get(), compareStateZ ) ); - const auto sc = computeState( tc, z, *state, it ); +const LHCb::State* TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location loc, + std::ptrdiff_t position ) const { + LHCb::State state{loc} ; + state.setZ( z ); + if ( position < 0 ) { + position = std::distance( tc.states().cbegin(), std::lower_bound( tc.states().cbegin(), tc.states().cend(), + &state, compareStateZ ) ); + } + const auto sc = computeState( tc, z, state, position ); if ( !sc ) return nullptr; - state->setLocation( loc ); + state.setLocation( loc ); if ( msgLevel( MSG::DEBUG ) ) { debug() << "Adding state to track cache: key=" << tc.track().key() << " " << z << " " << loc << endmsg; } - return tc.insertState( it, std::move( state ) ); + return tc.insertState( position, std::move( state ) ); } -StatusCode TrackStateProvider::computeState( const TrackCache& tc, double z, LHCb::State& state, - const LHCb::TrackTraj::StateContainer::const_iterator& position ) const { +StatusCode TrackStateProvider::computeState( const TrackCache& tc, // + const double z, // + LHCb::State& state, // + std::ptrdiff_t position ) const { // in brunel, we simply use the interpolator. in davinci, we use the // extrapolator, and we take some control over material corrections. const auto& track = tc.track(); - if ( track.fitResult() && !track.fitResult()->nodes().empty() && track.fitStatus() == LHCb::Track::Fitted ) { + const auto* fit = track.fitResult(); + if ( fit && !fit->nodes().empty() && track.fitStatus() == LHCb::Track::FitStatus::Fitted ) { return m_interpolator->interpolate( track, z, state ); } // locate the states surrounding this z position const auto& refstates = tc.states(); state.setZ( z ); - auto it = position; + auto it = std::next( refstates.begin(), position ); bool applyMaterialCorrections = false; if ( it == refstates.end() ) { state = *refstates.back(); } else if ( it == refstates.begin() || z < tc.zFirstMeasurement() ) { state = **it; // if we extrapolate from ClosestToBeam, we don't apply mat corrections. - applyMaterialCorrections = ( state.location() != LHCb::State::ClosestToBeam && m_applyMaterialCorrections ); + applyMaterialCorrections = + ( state.location() != LHCb::State::Location::ClosestToBeam && m_applyMaterialCorrections ); } else { // take the closest state. auto prev = std::prev( it ); @@ -394,10 +452,12 @@ StatusCode TrackStateProvider::computeState( const TrackCache& tc, double z, LHC LHCb::StateVector statevec( state.stateVector(), state.z() ); Gaudi::TrackMatrix transmat; - auto sc = m_extrapolator->propagate( statevec, z, &transmat ); - state.setState( statevec ); - state.setCovariance( LHCb::Math::Similarity( transmat, state.covariance() ) ); + auto sc = m_extrapolator->propagate( statevec, z, &transmat ); + if ( LIKELY( sc.isSuccess() ) ) { + state.setState( statevec ); + state.setCovariance( LHCb::Math::Similarity( transmat, state.covariance() ) ); + } return sc; } @@ -406,26 +466,27 @@ StatusCode TrackStateProvider::computeState( const TrackCache& tc, double z, LHC //============================================================================= TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Track& track ) const { - if ( msgLevel( MSG::DEBUG ) ) + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Creating track cache for track: key=" << track.key() << " hashID=" << key << " " << track.states().size() << endmsg; - + } + // create a new entry in the cache TrackCache tc{track} ; // make sure downstream tracks have a few ref states before the first measurement. if ( track.type() == LHCb::Track::Downstream ) { - for ( const auto& loc : {std::make_pair( LHCb::State::EndRich1, StateParameters::ZEndRich1 ), - std::make_pair( LHCb::State::BegRich1, StateParameters::ZBegRich1 ), - std::make_pair( LHCb::State::EndVelo, StateParameters::ZEndVelo )} ) { + for ( const auto& loc : {std::pair{LHCb::State::Location::EndRich1, StateParameters::ZEndRich1 }, + std::pair{LHCb::State::Location::BegRich1, StateParameters::ZBegRich1 }, + std::pair{LHCb::State::Location::EndVelo, StateParameters::ZEndVelo }} ) { if ( !track.stateAt( loc.first ) ) { addState( tc, loc.second, loc.first ); } } } // make sure all tracks (incl. Downstream) get assigned a state at // the beamline. this is useful for the trajectory approximation. - if ( ( track.hasVelo() || track.hasTT() ) && track.firstState().location() != LHCb::State::ClosestToBeam && - !track.stateAt( LHCb::State::ClosestToBeam ) ) { + if ( ( track.hasVelo() || track.hasTT() ) && track.firstState().location() != LHCb::State::Location::ClosestToBeam && + !track.stateAt( LHCb::State::Location::ClosestToBeam ) ) { // compute poca of first state with z-axis const auto& vec = track.firstState().stateVector(); // check on division by zero (track parallel to beam line!) @@ -436,7 +497,7 @@ TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Tra if ( dz < -10 * Gaudi::Units::cm ) { auto z = track.firstState().z() + dz; if ( z > -100 * Gaudi::Units::cm ) { // beginning of velo - addState( tc, z, LHCb::State::ClosestToBeam ); + addState( tc, z, LHCb::State::Location::ClosestToBeam ); } } } @@ -446,9 +507,9 @@ TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Tra // at the beamline and at rich2. For the trajectory approximation // used in DTF for Long-Long Ks, this is not enough. Add a state at // the end of the Velo. - if ( !track.checkFlag( LHCb::Track::Backward ) && track.hasVelo() && - !track.stateAt( LHCb::State::FirstMeasurement ) ) { - addState( tc, StateParameters::ZEndVelo, LHCb::State::EndVelo ); + if ( !track.checkFlag( LHCb::Track::Flags::Backward ) && track.hasVelo() && + !track.stateAt( LHCb::State::Location::FirstMeasurement ) ) { + addState( tc, StateParameters::ZEndVelo, LHCb::State::Location::EndVelo ); } return tc; @@ -458,15 +519,12 @@ TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Tra // retrieve TrackCache entry for a given track (from the cache) //============================================================================= TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { - // get the cache from the stack auto key = trackID( track ); auto& m_trackcache = trackcache() ; auto it = m_trackcache.find( key ); if ( it == m_trackcache.end() ) { - auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); - it = ret.first; + auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); + it = ret.first; } return it->second; } - -const LHCb::TrackTraj* TrackStateProvider::trajectory( const LHCb::Track& track ) const { return &cache( track ); } -- GitLab From a32ff7a04dc76dd49e934a3df9457077edf759cf Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Mon, 15 Jun 2020 13:56:31 +0200 Subject: [PATCH 09/19] apply lb-format; remove unnecessary constructor --- .../src/TrackStateProvider.cpp | 164 ++++++++---------- 1 file changed, 75 insertions(+), 89 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index 94ba82cfb9e..b2eadd9d8ae 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -11,8 +11,8 @@ // from Gaudi #include "GaudiAlg/GaudiTool.h" -#include "GaudiKernel/ToolHandle.h" #include "GaudiKernel/DataObjectHandle.h" +#include "GaudiKernel/ToolHandle.h" // from TrackInterfaces #include "TrackInterfaces/ITrackExtrapolator.h" @@ -35,8 +35,8 @@ #include <memory> #include <mutex> #include <numeric> -#include <unordered_map> #include <thread> +#include <unordered_map> // boost #include "boost/functional/hash.hpp" @@ -65,7 +65,7 @@ namespace { // key is based on track memory addresses TkCacheKey id = TkCacheKey( &track ); boost::hash_combine( id, track.fitResult() ); - if( !track.lhcbIDs().empty() ) { + if ( !track.lhcbIDs().empty() ) { boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); } @@ -106,10 +106,10 @@ namespace { }; const LHCb::State* TrackCache::insertState( std::ptrdiff_t pos, LHCb::State&& o_state ) { - + // take ownership of state - m_ownedstates.push_back( o_state ) ; - auto state = &(m_ownedstates.back()) ; + m_ownedstates.push_back( o_state ); + auto state = &( m_ownedstates.back() ); // get the vector with states auto& refstates = refStates(); @@ -127,80 +127,66 @@ namespace { } /// Type for container of track caches - using TrackCaches = std::unordered_map<TkCacheKey,TrackCache> ; + using TrackCaches = std::unordered_map<TkCacheKey, TrackCache>; // Need to solve two problems at once // * there can be multiple tool instances sharing the same cache // * there can be multiple threads accessing different caches in the event store // I'd like all that logic to be inside the handle - + // wrapper class to hold object with 'subscribed' pointers - template<typename T> - class SubscribedObject : public T - { + template <typename T> + class SubscribedObject : public T { public: - // need to implement a move constructor and forbid a copy constructor - //using T::T ; - SubscribedObject() = default ; - SubscribedObject( const SubscribedObject& ) = delete ; + // add a move constructor and forbid a copy constructor + SubscribedObject() = default; + SubscribedObject( const SubscribedObject& ) = delete; SubscribedObject( SubscribedObject&& rhs ) : T{rhs}, m_subscribers{rhs.m_subscribers} { - for( auto& p : m_subscribers ) *p = this ; + for ( auto& p : m_subscribers ) *p = this; } ~SubscribedObject() { - for( auto& p : m_subscribers ) *p = nullptr ; - } - void addsubscriber( T*& p) { - m_subscribers.push_back( &p ) ; - p = this ; + for ( auto& p : m_subscribers ) *p = nullptr; + } + void addsubscriber( T*& p ) { + m_subscribers.push_back( &p ); + p = this; } + private: - std::vector< T** > m_subscribers ; - } ; + std::vector<T**> m_subscribers; + }; // wrapper to hold Subscribed objects with fast pointers that depend on the thread - template<typename T> - class ThreadedAnyDataObjectSmartHandle : public DataObjectHandle<AnyDataWrapper<SubscribedObject<T> > > - { + template <typename T> + class ThreadedAnyDataObjectSmartHandle : public DataObjectHandle<AnyDataWrapper<SubscribedObject<T>>> { public: - using BASE = DataObjectHandle<AnyDataWrapper<SubscribedObject<T> > > ; - //using BASE::BASE ; - - /// Autodeclaring constructor with property name, mode, key and documentation. - /// @note the use std::enable_if is required to avoid ambiguities - template <class OWNER, class K, typename = std::enable_if_t<std::is_base_of_v<IProperty, OWNER>>> - ThreadedAnyDataObjectSmartHandle( OWNER* owner, Gaudi::DataHandle::Mode m, std::string name, const K& key = {}, - std::string doc = "" ) - : BASE{ key, m, owner } { - auto p = owner->declareProperty( std::move( name ), *this, std::move( doc ) ); - p->template setOwnerType<OWNER>(); - } - - T* get( std::mutex& mutex) const { + using BASE = DataObjectHandle<AnyDataWrapper<SubscribedObject<T>>>; + using BASE::BASE; + + T* get( std::mutex& mutex ) const { // add an entry to the map, if it doesn't exist yet. this should only happen once per thread. - const auto threadid = std::this_thread::get_id() ; - auto it = m_threadcachemap.find(threadid) ; - if( it == m_threadcachemap.end() ) { - std::scoped_lock lock{ mutex } ; - auto insertion = m_threadcachemap.insert( std::pair{threadid, nullptr} ) ; - it = insertion.first ; + const auto threadid = std::this_thread::get_id(); + auto it = m_threadcachemap.find( threadid ); + if ( it == m_threadcachemap.end() ) { + std::scoped_lock lock{mutex}; + auto insertion = m_threadcachemap.insert( std::pair{threadid, nullptr} ); + it = insertion.first; } - if( !it->second ) { - // first check if it is not already in the event store - auto* obj = this->getIfExists() ; - // if not, add it. this should happen only once per event per thread. - if( !obj ) { - obj = const_cast<SubscribedObject<T>*>(this->put(SubscribedObject<T>{})); - } - obj->addsubscriber( it->second ) ; + if ( !it->second ) { + // first check if it is not already in the event store + auto* obj = this->getIfExists(); + // if not, add it. this should happen only once per event per thread. + if ( !obj ) { obj = const_cast<SubscribedObject<T>*>( this->put( SubscribedObject<T>{} ) ); } + obj->addsubscriber( it->second ); } - return const_cast<T*>(it->second) ; + return const_cast<T*>( it->second ); } - + private: - mutable std::unordered_map< std::thread::id, T* > m_threadcachemap ; - } ; - -} //namespace + mutable std::unordered_map<std::thread::id, T*> m_threadcachemap; + }; + +} // namespace class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider> { @@ -240,27 +226,26 @@ public: void clearCache( const LHCb::Track& track ) const override; private: - StatusCode computeState( const TrackCache& tc, // const double z, // LHCb::State& state, // std::ptrdiff_t position ) const; - + const LHCb::State* addState( TrackCache& tc, // double z, // LHCb::State::Location loc = LHCb::State::Location::LocationUnknown, std::ptrdiff_t position = -1 ) const; - + /// Get the track cache from the event store TrackCaches& trackcache() const { - return const_cast<TrackCaches&>(*m_caches.get(m_mutex) ) ; - //auto* obj = m_caches.getIfExists(); - //return const_cast<TrackCaches&>(obj ? *obj : *(m_caches.put( TrackCaches{} ))) ; + return const_cast<TrackCaches&>( *m_caches.get( m_mutex ) ); + // auto* obj = m_caches.getIfExists(); + // return const_cast<TrackCaches&>(obj ? *obj : *(m_caches.put( TrackCaches{} ))) ; } - + /// Create a cache entry TrackCache createCacheEntry( const TkCacheKey key, const LHCb::Track& track ) const; - + /// Retrieve a single cache entry TrackCache& cache( const LHCb::Track& track ) const; @@ -269,7 +254,7 @@ private: // key is based on track memory addresses TkCacheKey id = TkCacheKey( &track ); boost::hash_combine( id, track.fitResult() ); - if( !track.lhcbIDs().empty() ) { + if ( !track.lhcbIDs().empty() ) { boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); } @@ -277,10 +262,11 @@ private: } private: - mutable std::mutex m_mutex ; - mutable ThreadedAnyDataObjectSmartHandle<TrackCaches> m_caches { this, Gaudi::DataHandle::Writer, "CacheLocation", "TrackStateProviderCache" }; - ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; - ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; + mutable std::mutex m_mutex; + mutable ThreadedAnyDataObjectSmartHandle<TrackCaches> m_caches{this, Gaudi::DataHandle::Writer, "CacheLocation", + "TrackStateProviderCache"}; + ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; + ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; Gaudi::Property<bool> m_applyMaterialCorrections{this, "ApplyMaterialCorrections", true}; Gaudi::Property<double> m_linearPropagationTolerance{this, "LinearPropagationTolerance", 1.0 * Gaudi::Units::mm}; @@ -327,12 +313,12 @@ StatusCode TrackStateProvider::finalize() { //============================================================================= void TrackStateProvider::clearCache() const { - auto& m_trackcache = trackcache() ; + auto& m_trackcache = trackcache(); if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; counter( "Number of tracks seen" ) += m_trackcache.size(); counter( "Number of states added" ) += - std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, - []( auto n, const auto& p ) { return n + p.second.numOwnedStates(); } ); + std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, + []( auto n, const auto& p ) { return n + p.second.numOwnedStates(); } ); m_trackcache.clear(); } @@ -342,8 +328,8 @@ void TrackStateProvider::clearCache() const { void TrackStateProvider::clearCache( const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; - auto& m_trackcache = trackcache() ; - auto it = m_trackcache.find( trackID( track ) ); + auto& m_trackcache = trackcache(); + auto it = m_trackcache.find( trackID( track ) ); if ( it != m_trackcache.end() ) m_trackcache.erase( it ); } @@ -401,11 +387,11 @@ StatusCode TrackStateProvider::state( LHCb::State& state, // const LHCb::State* TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location loc, std::ptrdiff_t position ) const { - LHCb::State state{loc} ; + LHCb::State state{loc}; state.setZ( z ); if ( position < 0 ) { - position = std::distance( tc.states().cbegin(), std::lower_bound( tc.states().cbegin(), tc.states().cend(), - &state, compareStateZ ) ); + position = std::distance( tc.states().cbegin(), + std::lower_bound( tc.states().cbegin(), tc.states().cend(), &state, compareStateZ ) ); } const auto sc = computeState( tc, z, state, position ); if ( !sc ) return nullptr; @@ -470,15 +456,15 @@ TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Tra debug() << "Creating track cache for track: key=" << track.key() << " hashID=" << key << " " << track.states().size() << endmsg; } - + // create a new entry in the cache - TrackCache tc{track} ; + TrackCache tc{track}; // make sure downstream tracks have a few ref states before the first measurement. if ( track.type() == LHCb::Track::Downstream ) { - for ( const auto& loc : {std::pair{LHCb::State::Location::EndRich1, StateParameters::ZEndRich1 }, - std::pair{LHCb::State::Location::BegRich1, StateParameters::ZBegRich1 }, - std::pair{LHCb::State::Location::EndVelo, StateParameters::ZEndVelo }} ) { + for ( const auto& loc : {std::pair{LHCb::State::Location::EndRich1, StateParameters::ZEndRich1}, + std::pair{LHCb::State::Location::BegRich1, StateParameters::ZBegRich1}, + std::pair{LHCb::State::Location::EndVelo, StateParameters::ZEndVelo}} ) { if ( !track.stateAt( loc.first ) ) { addState( tc, loc.second, loc.first ); } } } @@ -519,9 +505,9 @@ TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Tra // retrieve TrackCache entry for a given track (from the cache) //============================================================================= TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { - auto key = trackID( track ); - auto& m_trackcache = trackcache() ; - auto it = m_trackcache.find( key ); + auto key = trackID( track ); + auto& m_trackcache = trackcache(); + auto it = m_trackcache.find( key ); if ( it == m_trackcache.end() ) { auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); it = ret.first; -- GitLab From 79be6790be72ca7c8a1f3d6ceaeefe619094df88 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Mon, 15 Jun 2020 16:55:01 +0200 Subject: [PATCH 10/19] removed duplicate function --- Tr/TrackExtrapolators/src/TrackStateProvider.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index b2eadd9d8ae..f9e7269d6f6 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -249,18 +249,6 @@ private: /// Retrieve a single cache entry TrackCache& cache( const LHCb::Track& track ) const; - /// Create the key for a given track - inline TkCacheKey trackID( const LHCb::Track& track ) const { - // key is based on track memory addresses - TkCacheKey id = TkCacheKey( &track ); - boost::hash_combine( id, track.fitResult() ); - if ( !track.lhcbIDs().empty() ) { - boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); - boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); - } - return id; - } - private: mutable std::mutex m_mutex; mutable ThreadedAnyDataObjectSmartHandle<TrackCaches> m_caches{this, Gaudi::DataHandle::Writer, "CacheLocation", -- GitLab From 36daf0484a90b56ed5b1cafc8dce7483cee37b30 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Tue, 9 Jun 2020 17:40:10 +0200 Subject: [PATCH 11/19] added reset of TrackStateProvider cache to TrackMasterFitter --- Tr/TrackFitter/src/TrackMasterFitter.cpp | 3 +++ Tr/TrackFitter/src/TrackMasterFitter.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/Tr/TrackFitter/src/TrackMasterFitter.cpp b/Tr/TrackFitter/src/TrackMasterFitter.cpp index b439db15ead..de266d2cf26 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.cpp +++ b/Tr/TrackFitter/src/TrackMasterFitter.cpp @@ -263,6 +263,9 @@ StatusCode TrackMasterFitter::fit_r( Track& track, std::any& accelCache, LHCb::P // any track that doesnt make it to the end is failed track.setFitStatus( Track::FitStatus::FitFailed ); + // make sure to reset the stateprovider cache, no matter what happens next. + const_cast<ITrackStateProvider&>(*m_stateProvider).clearCache( track ) ; + // create the KalmanFitResult if it doesn't exist yet LHCb::KalmanFitResult* kalfitresult = dynamic_cast<LHCb::KalmanFitResult*>( track.fitResult() ); if ( !kalfitresult ) { diff --git a/Tr/TrackFitter/src/TrackMasterFitter.h b/Tr/TrackFitter/src/TrackMasterFitter.h index 616cf30c6f6..88f384d45b7 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.h +++ b/Tr/TrackFitter/src/TrackMasterFitter.h @@ -24,6 +24,7 @@ #include "TrackInterfaces/ITrackFitter.h" #include "TrackInterfaces/ITrackKalmanFilter.h" #include "TrackInterfaces/ITrackProjectorSelector.h" +#include "TrackInterfaces/ITrackStateProvider.h" // Forward declarations struct ITrackManipulator; @@ -114,6 +115,7 @@ private: ToolHandle<IMeasurementProvider> m_measProvider; ToolHandle<IMaterialLocator> m_materialLocator; ToolHandle<ITrackProjectorSelector> m_projectorSelector; + PublicToolHandle<ITrackStateProvider> m_stateProvider{this,"StateProvider","TrackStateProvider"} ; private: // job options -- GitLab From 32e6d21c32bc1f71f9d4a4dca0d15a945100bc8d Mon Sep 17 00:00:00 2001 From: Gitlab CI <noreply@cern.ch> Date: Tue, 9 Jun 2020 15:41:37 +0000 Subject: [PATCH 12/19] Fixed formatting patch generated by https://gitlab.cern.ch/lhcb/Rec/-/jobs/8729478 --- Tr/TrackFitter/src/TrackMasterFitter.cpp | 2 +- Tr/TrackFitter/src/TrackMasterFitter.h | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Tr/TrackFitter/src/TrackMasterFitter.cpp b/Tr/TrackFitter/src/TrackMasterFitter.cpp index de266d2cf26..c570044ec0a 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.cpp +++ b/Tr/TrackFitter/src/TrackMasterFitter.cpp @@ -264,7 +264,7 @@ StatusCode TrackMasterFitter::fit_r( Track& track, std::any& accelCache, LHCb::P track.setFitStatus( Track::FitStatus::FitFailed ); // make sure to reset the stateprovider cache, no matter what happens next. - const_cast<ITrackStateProvider&>(*m_stateProvider).clearCache( track ) ; + const_cast<ITrackStateProvider&>( *m_stateProvider ).clearCache( track ); // create the KalmanFitResult if it doesn't exist yet LHCb::KalmanFitResult* kalfitresult = dynamic_cast<LHCb::KalmanFitResult*>( track.fitResult() ); diff --git a/Tr/TrackFitter/src/TrackMasterFitter.h b/Tr/TrackFitter/src/TrackMasterFitter.h index 88f384d45b7..b77a3c316a6 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.h +++ b/Tr/TrackFitter/src/TrackMasterFitter.h @@ -109,13 +109,13 @@ private: } private: - ToolHandle<ITrackExtrapolator> m_extrapolator; ///< extrapolator - ToolHandle<ITrackExtrapolator> m_veloExtrapolator; ///< extrapolator for Velo-only tracks - ToolHandle<ITrackKalmanFilter> m_trackNodeFitter; ///< delegate to actual track fitter (which fits from nodes) - ToolHandle<IMeasurementProvider> m_measProvider; - ToolHandle<IMaterialLocator> m_materialLocator; - ToolHandle<ITrackProjectorSelector> m_projectorSelector; - PublicToolHandle<ITrackStateProvider> m_stateProvider{this,"StateProvider","TrackStateProvider"} ; + ToolHandle<ITrackExtrapolator> m_extrapolator; ///< extrapolator + ToolHandle<ITrackExtrapolator> m_veloExtrapolator; ///< extrapolator for Velo-only tracks + ToolHandle<ITrackKalmanFilter> m_trackNodeFitter; ///< delegate to actual track fitter (which fits from nodes) + ToolHandle<IMeasurementProvider> m_measProvider; + ToolHandle<IMaterialLocator> m_materialLocator; + ToolHandle<ITrackProjectorSelector> m_projectorSelector; + PublicToolHandle<ITrackStateProvider> m_stateProvider{this, "StateProvider", "TrackStateProvider"}; private: // job options -- GitLab From 6951875d6313f8e5a0d0111d9b40c8af842d0689 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Fri, 12 Jun 2020 11:58:53 +0200 Subject: [PATCH 13/19] made access to cache const --- Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h b/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h index 752d2c526e4..59bca3ab338 100644 --- a/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h +++ b/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h @@ -53,9 +53,9 @@ struct ITrackStateProvider : extend_interfaces<IAlgTool> { virtual const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const = 0; /// Clear the cache - virtual void clearCache() = 0; + virtual void clearCache() const = 0; /// Clear the cache for a particular track - virtual void clearCache( const LHCb::Track& track ) = 0; + virtual void clearCache( const LHCb::Track& track ) const = 0; }; #endif // TRACKINTERFACES_ITRACKSTATEPROVIDER_H -- GitLab From 68dc90e26cda1fda08f2bf306f5938265b49bc15 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Fri, 12 Jun 2020 12:02:48 +0200 Subject: [PATCH 14/19] removed const_cast --- Tr/TrackFitter/src/TrackMasterFitter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tr/TrackFitter/src/TrackMasterFitter.cpp b/Tr/TrackFitter/src/TrackMasterFitter.cpp index c570044ec0a..6c1b14d0e62 100644 --- a/Tr/TrackFitter/src/TrackMasterFitter.cpp +++ b/Tr/TrackFitter/src/TrackMasterFitter.cpp @@ -264,7 +264,7 @@ StatusCode TrackMasterFitter::fit_r( Track& track, std::any& accelCache, LHCb::P track.setFitStatus( Track::FitStatus::FitFailed ); // make sure to reset the stateprovider cache, no matter what happens next. - const_cast<ITrackStateProvider&>( *m_stateProvider ).clearCache( track ); + m_stateProvider->clearCache( track ); // create the KalmanFitResult if it doesn't exist yet LHCb::KalmanFitResult* kalfitresult = dynamic_cast<LHCb::KalmanFitResult*>( track.fitResult() ); -- GitLab From f1d62602a7432ccfffd49f897dfc708619e1add0 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Fri, 12 Jun 2020 13:07:36 +0200 Subject: [PATCH 15/19] made the trackid based on hash; removed check that track is in TES; added mutex locks to try and make code thread safe --- .../src/TrackStateProvider.cpp | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index a15d2edcf9f..ebae06bcde1 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -88,17 +88,17 @@ public: const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override; /// Clear the cache - void clearCache() override; + void clearCache() const override; /// Clear the cache for a particular track - void clearCache( const LHCb::Track& track ) override; + void clearCache( const LHCb::Track& track ) const override; /// incident service handle void handle( const Incident& incident ) override; private: /// Type for cache key - using TkCacheKey = std::uintptr_t; + using TkCacheKey = std::size_t; StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; @@ -115,15 +115,16 @@ private: /// Create the key for a given track inline TkCacheKey trackID( const LHCb::Track& track ) const { - // warn if operating on a track with no parent container. - if ( !track.parent() ) { - Error( "Track has no parent container. TrackStateProvider cache might be unreliable.." ).ignore(); - } // key is based on track memory addresses - return TkCacheKey( &track ); + TkCacheKey id = TkCacheKey(&track) ; + boost::hash_combine(id, track.fitResult() ) ; + boost::hash_combine(id, track.lhcbIDs().front().lhcbID() ) ; + boost::hash_combine(id, track.lhcbIDs().back().lhcbID() ) ; + return id ; } private: + mutable std::mutex m_cachelock ; mutable std::unordered_map<TkCacheKey, std::unique_ptr<::TrackCache>> m_trackcache; ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; @@ -261,25 +262,29 @@ void TrackStateProvider::handle( const Incident& /* incident */ ) { // Clear cache (and update some counters) //============================================================================= -void TrackStateProvider::clearCache() { +void TrackStateProvider::clearCache() const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; counter( "Number of tracks seen" ) += m_trackcache.size(); if ( !m_trackcache.empty() ) { + std::lock_guard lockguard( m_cachelock ) ; counter( "Number of states added" ) += std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, []( auto n, const auto& p ) { return n + p.second->numOwnedStates(); } ); + m_trackcache.clear(); } - m_trackcache.clear(); } //============================================================================= // Clear cache for a given track //============================================================================= -void TrackStateProvider::clearCache( const LHCb::Track& track ) { +void TrackStateProvider::clearCache( const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; auto it = m_trackcache.find( trackID( track ) ); - if ( it != m_trackcache.end() ) m_trackcache.erase( it ); + if ( it != m_trackcache.end() ) { + std::lock_guard lockguard( m_cachelock ) ; + m_trackcache.erase( it ); + } } //============================================================================= @@ -342,6 +347,7 @@ TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location lo if ( msgLevel( MSG::DEBUG ) ) { debug() << "Adding state to track cache: key=" << tc.track().key() << " " << z << " " << loc << endmsg; } + std::lock_guard lockguard( m_cachelock ) ; return tc.insertState( it, std::move( state ) ); } @@ -444,6 +450,7 @@ TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { auto key = trackID( track ); auto it = m_trackcache.find( key ); if ( it == m_trackcache.end() ) { + std::lock_guard lockguard( m_cachelock ) ; auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); it = ret.first; } -- GitLab From a094eefa0a4d10fd18f05f34ab291d59b5ea50fc Mon Sep 17 00:00:00 2001 From: Gitlab CI <noreply@cern.ch> Date: Fri, 12 Jun 2020 11:09:54 +0000 Subject: [PATCH 16/19] Fixed formatting patch generated by https://gitlab.cern.ch/lhcb/Rec/-/jobs/8778419 --- .../src/TrackStateProvider.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index ebae06bcde1..f4648b4bb86 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -116,15 +116,15 @@ private: /// Create the key for a given track inline TkCacheKey trackID( const LHCb::Track& track ) const { // key is based on track memory addresses - TkCacheKey id = TkCacheKey(&track) ; - boost::hash_combine(id, track.fitResult() ) ; - boost::hash_combine(id, track.lhcbIDs().front().lhcbID() ) ; - boost::hash_combine(id, track.lhcbIDs().back().lhcbID() ) ; - return id ; + TkCacheKey id = TkCacheKey( &track ); + boost::hash_combine( id, track.fitResult() ); + boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); + boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); + return id; } private: - mutable std::mutex m_cachelock ; + mutable std::mutex m_cachelock; mutable std::unordered_map<TkCacheKey, std::unique_ptr<::TrackCache>> m_trackcache; ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; @@ -266,7 +266,7 @@ void TrackStateProvider::clearCache() const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; counter( "Number of tracks seen" ) += m_trackcache.size(); if ( !m_trackcache.empty() ) { - std::lock_guard lockguard( m_cachelock ) ; + std::lock_guard lockguard( m_cachelock ); counter( "Number of states added" ) += std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, []( auto n, const auto& p ) { return n + p.second->numOwnedStates(); } ); @@ -282,7 +282,7 @@ void TrackStateProvider::clearCache( const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; auto it = m_trackcache.find( trackID( track ) ); if ( it != m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ) ; + std::lock_guard lockguard( m_cachelock ); m_trackcache.erase( it ); } } @@ -347,7 +347,7 @@ TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location lo if ( msgLevel( MSG::DEBUG ) ) { debug() << "Adding state to track cache: key=" << tc.track().key() << " " << z << " " << loc << endmsg; } - std::lock_guard lockguard( m_cachelock ) ; + std::lock_guard lockguard( m_cachelock ); return tc.insertState( it, std::move( state ) ); } @@ -450,9 +450,9 @@ TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { auto key = trackID( track ); auto it = m_trackcache.find( key ); if ( it == m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ) ; - auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); - it = ret.first; + std::lock_guard lockguard( m_cachelock ); + auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); + it = ret.first; } return *it->second; } -- GitLab From a09ba33db4222191068ae3a122038fbf46f23d76 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Mon, 15 Jun 2020 08:34:11 +0200 Subject: [PATCH 17/19] Make trackstateprovider use TES --- .../src/TrackStateProvider.cpp | 235 +++++++++--------- 1 file changed, 119 insertions(+), 116 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index f4648b4bb86..9edbb12fa62 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -8,10 +8,10 @@ * granted to it by virtue of its status as an Intergovernmental Organization * * or submit itself to any jurisdiction. * \*****************************************************************************/ -// Include files -// ------------- + // from Gaudi #include "GaudiAlg/GaudiTool.h" +#include "GaudiKernel/DataObjectHandle.h" #include "GaudiKernel/IIncidentListener.h" #include "GaudiKernel/IIncidentSvc.h" #include "GaudiKernel/ToolHandle.h" @@ -53,95 +53,29 @@ // forward declarations namespace { - class TrackCache; -} - -class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider, IIncidentListener> { - -public: - /// Standard constructor - TrackStateProvider( const std::string& type, const std::string& name, const IInterface* parent ); - - /// initialize - StatusCode initialize() override; - - /// initialize - StatusCode finalize() override; - - /// Compute the state of the track at position z. The third - /// argument is the tolerance: if an existing state is found within - /// a z-distance 'tolerance', that state is returned. - /// If there are 'fitnodes' on the track (e.g. in Brunel), this - /// method will use interpolation. If there are no fitnodes (e.g. in - /// DaVinci) the method will use extrapolation. In that case the - /// answer is only correct outside the measurement range. - StatusCode state( LHCb::State& astate, const LHCb::Track& track, double z, double ztolerance ) const override; - - /// Compute state using cached trajectory - StatusCode stateFromTrajectory( LHCb::State& state, const LHCb::Track& track, double z ) const override { - const auto traj = trajectory( track ); - if ( traj ) { state = traj->state( z ); } - return ( traj ? StatusCode::SUCCESS : StatusCode::FAILURE ); - } - - /// Retrieve the cached trajectory - const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override; - - /// Clear the cache - void clearCache() const override; - /// Clear the cache for a particular track - void clearCache( const LHCb::Track& track ) const override; - - /// incident service handle - void handle( const Incident& incident ) override; - -private: /// Type for cache key - using TkCacheKey = std::size_t; - - StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, - const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; - - const LHCb::State* - addState( TrackCache& tc, double z, LHCb::State::Location loc = LHCb::State::LocationUnknown, - boost::optional<LHCb::TrackTraj::StateContainer::const_iterator> position = boost::none ) const; - - /// Create a cache entry - std::unique_ptr<TrackCache> createCacheEntry( const TkCacheKey key, const LHCb::Track& track ) const; + using TkCacheKey = std::uintptr_t; - /// Retrieve a cache entry - TrackCache& cache( const LHCb::Track& track ) const; + /// compare states by Z position + inline constexpr auto compareStateZ = []( const LHCb::State* lhs, const LHCb::State* rhs ) { + return lhs->z() < rhs->z(); + }; /// Create the key for a given track - inline TkCacheKey trackID( const LHCb::Track& track ) const { - // key is based on track memory addresses - TkCacheKey id = TkCacheKey( &track ); - boost::hash_combine( id, track.fitResult() ); - boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); - boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); - return id; + inline TkCacheKey trackID( const LHCb::Track& track ) { + TkCacheKey tkid = TkCacheKey( &track ); + if ( track.fitResult() ) { boost::hash_combine( tkid, track.fitResult() ); } + if ( track.parent() ) { boost::hash_combine( tkid, track.parent() ); } + // How many IDs are needed to 'guarantee' uniqueness in hash ?? + // for ( const auto id : track.lhcbIDs() ) { boost::hash_combine( tkid, id.lhcbID() ); } + if ( LIKELY( !track.lhcbIDs().empty() ) ) { + boost::hash_combine( tkid, track.lhcbIDs().front().lhcbID() ); + boost::hash_combine( tkid, track.lhcbIDs().back().lhcbID() ); + } + return tkid; } -private: - mutable std::mutex m_cachelock; - mutable std::unordered_map<TkCacheKey, std::unique_ptr<::TrackCache>> m_trackcache; - ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; - ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; - - Gaudi::Property<bool> m_applyMaterialCorrections{this, "ApplyMaterialCorrections", true}; - Gaudi::Property<double> m_linearPropagationTolerance{this, "LinearPropagationTolerance", 1.0 * Gaudi::Units::mm}; - Gaudi::Property<bool> m_cacheStatesOnDemand{this, "CacheStatesOnDemand", false}; -}; - -/**********************************************************************************************/ - -namespace { - - constexpr struct compareStateZ_t { - bool operator()( const LHCb::State* lhs, const LHCb::State* rhs ) const { return lhs->z() < rhs->z(); } - } compareStateZ{}; - // The TrackCache is basically just an 'extendable' TrackTraj class TrackCache final : public LHCb::TrackTraj { @@ -151,6 +85,7 @@ namespace { double m_zFirstMeasurement{-9999}; public: + /// Constructor from a track TrackCache( const LHCb::Track& track ) : LHCb::TrackTraj( track ), m_track( &track ) { const auto state = track.stateAt( LHCb::State::FirstMeasurement ); m_zFirstMeasurement = ( state ? state->z() : -9999 ); @@ -212,8 +147,86 @@ namespace { return state; } + + /// Type for cache + using TrackCaches = std::unordered_map<TkCacheKey, TrackCache>; + } // namespace +class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider, IIncidentListener> { + +public: + /// Standard constructor + TrackStateProvider( const std::string& type, const std::string& name, const IInterface* parent ); + + /// initialize + StatusCode initialize() override; + + /// initialize + StatusCode finalize() override; + + /// Compute the state of the track at position z. The third + /// argument is the tolerance: if an existing state is found within + /// a z-distance 'tolerance', that state is returned. + /// If there are 'fitnodes' on the track (e.g. in Brunel), this + /// method will use interpolation. If there are no fitnodes (e.g. in + /// DaVinci) the method will use extrapolation. In that case the + /// answer is only correct outside the measurement range. + StatusCode state( LHCb::State& astate, const LHCb::Track& track, double z, double ztolerance ) const override; + + /// Compute state using cached trajectory + StatusCode stateFromTrajectory( LHCb::State& state, const LHCb::Track& track, double z ) const override { + const auto traj = trajectory( track ); + if ( traj ) { state = traj->state( z ); } + return ( traj ? StatusCode::SUCCESS : StatusCode::FAILURE ); + } + + /// Retrieve the cached trajectory + const LHCb::TrackTraj* trajectory( const LHCb::Track& track ) const override { return &cache( track ); } + + /// Clear the cache + void clearCache() const override; + + /// Clear the cache for a particular track + void clearCache( const LHCb::Track& track ) const override; + + /// incident service handle + void handle( const Incident& incident ) override; + +private: + StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, + const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; + + const LHCb::State* + addState( TrackCache& tc, double z, LHCb::State::Location loc = LHCb::State::LocationUnknown, + boost::optional<LHCb::TrackTraj::StateContainer::const_iterator> position = boost::none ) const; + + /// Get the track cache from the event store + TrackCaches& trackcache() const { + auto* obj = m_caches.getIfExists(); + return const_cast<TrackCaches&>( obj ? *obj : *( m_caches.put( TrackCaches{} ) ) ); + } + + /// Create a cache entry + TrackCache createCacheEntry( const TkCacheKey key, const LHCb::Track& track ) const; + + /// Retrieve a single cache entry + TrackCache& cache( const LHCb::Track& track ) const; + +private: + mutable DataObjectHandle<AnyDataWrapper<TrackCaches>> m_caches{this, Gaudi::DataHandle::Writer, "CacheLocation", + "TrackStateProviderCache"}; + + ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; + ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; + + Gaudi::Property<bool> m_applyMaterialCorrections{this, "ApplyMaterialCorrections", true}; + Gaudi::Property<double> m_linearPropagationTolerance{this, "LinearPropagationTolerance", 1.0 * Gaudi::Units::mm}; + Gaudi::Property<bool> m_cacheStatesOnDemand{this, "CacheStatesOnDemand", false}; +}; + +/**********************************************************************************************/ + DECLARE_COMPONENT( TrackStateProvider ) //============================================================================= @@ -234,7 +247,7 @@ StatusCode TrackStateProvider::initialize() { if ( sc.isSuccess() ) sc = m_extrapolator.retrieve(); if ( sc.isSuccess() ) sc = m_interpolator.retrieve(); // reset at the start of each event - incSvc()->addListener( this, IncidentType::BeginEvent ); + // incSvc()->addListener( this, IncidentType::BeginEvent ); return sc; } @@ -242,7 +255,7 @@ StatusCode TrackStateProvider::initialize() { // Finalize //============================================================================= StatusCode TrackStateProvider::finalize() { - clearCache(); + // clearCache(); m_extrapolator.release().ignore(); m_interpolator.release().ignore(); return GaudiTool::finalize(); @@ -263,15 +276,9 @@ void TrackStateProvider::handle( const Incident& /* incident */ ) { //============================================================================= void TrackStateProvider::clearCache() const { - if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; - counter( "Number of tracks seen" ) += m_trackcache.size(); - if ( !m_trackcache.empty() ) { - std::lock_guard lockguard( m_cachelock ); - counter( "Number of states added" ) += - std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, - []( auto n, const auto& p ) { return n + p.second->numOwnedStates(); } ); - m_trackcache.clear(); - } + auto& tc = trackcache(); + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Clearing cache. Size is " << tc.size() << "." << endmsg; } + tc.clear(); } //============================================================================= @@ -279,12 +286,10 @@ void TrackStateProvider::clearCache() const { //============================================================================= void TrackStateProvider::clearCache( const LHCb::Track& track ) const { - if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; - auto it = m_trackcache.find( trackID( track ) ); - if ( it != m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ); - m_trackcache.erase( it ); - } + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Clearing cache for track: key=" << track.key() << endmsg; } + auto& tc = trackcache(); + auto it = tc.find( trackID( track ) ); + if ( it != tc.end() ) { tc.erase( it ); } } //============================================================================= @@ -347,7 +352,6 @@ TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location lo if ( msgLevel( MSG::DEBUG ) ) { debug() << "Adding state to track cache: key=" << tc.track().key() << " " << z << " " << loc << endmsg; } - std::lock_guard lockguard( m_cachelock ); return tc.insertState( it, std::move( state ) ); } @@ -394,20 +398,20 @@ StatusCode TrackStateProvider::computeState( const TrackCache& tc, double z, LHC // retrieve the cache for a given track //============================================================================= -std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Track& track ) const { +TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Track& track ) const { if ( msgLevel( MSG::DEBUG ) ) debug() << "Creating track cache for track: key=" << track.key() << " hashID=" << key << " " << track.states().size() << endmsg; // create a new entry in the cache - auto tc = std::make_unique<TrackCache>( track ); + TrackCache tc{track}; // make sure downstream tracks have a few ref states before the first measurement. if ( track.type() == LHCb::Track::Downstream ) { for ( const auto& loc : {std::make_pair( LHCb::State::EndRich1, StateParameters::ZEndRich1 ), std::make_pair( LHCb::State::BegRich1, StateParameters::ZBegRich1 ), std::make_pair( LHCb::State::EndVelo, StateParameters::ZEndVelo )} ) { - if ( !track.stateAt( loc.first ) ) { addState( *tc, loc.second, loc.first ); } + if ( !track.stateAt( loc.first ) ) { addState( tc, loc.second, loc.first ); } } } @@ -425,7 +429,7 @@ std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key if ( dz < -10 * Gaudi::Units::cm ) { auto z = track.firstState().z() + dz; if ( z > -100 * Gaudi::Units::cm ) { // beginning of velo - addState( *tc, z, LHCb::State::ClosestToBeam ); + addState( tc, z, LHCb::State::ClosestToBeam ); } } } @@ -437,7 +441,7 @@ std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key // the end of the Velo. if ( !track.checkFlag( LHCb::Track::Backward ) && track.hasVelo() && !track.stateAt( LHCb::State::FirstMeasurement ) ) { - addState( *tc, StateParameters::ZEndVelo, LHCb::State::EndVelo ); + addState( tc, StateParameters::ZEndVelo, LHCb::State::EndVelo ); } return tc; @@ -447,14 +451,13 @@ std::unique_ptr<TrackCache> TrackStateProvider::createCacheEntry( TkCacheKey key // retrieve TrackCache entry for a given track (from the cache) //============================================================================= TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { - auto key = trackID( track ); - auto it = m_trackcache.find( key ); - if ( it == m_trackcache.end() ) { - std::lock_guard lockguard( m_cachelock ); - auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); - it = ret.first; + // get the cache from the stack + const auto key = trackID( track ); + auto& tc = trackcache(); + auto it = tc.find( key ); + if ( it == tc.end() ) { + auto ret = tc.emplace( key, createCacheEntry( key, track ) ); + it = ret.first; } - return *it->second; + return it->second; } - -const LHCb::TrackTraj* TrackStateProvider::trajectory( const LHCb::Track& track ) const { return &cache( track ); } -- GitLab From 36fb93fc53f362966f420325a18500510ded36b1 Mon Sep 17 00:00:00 2001 From: Chris Jones <jonesc@hep.phy.cam.ac.uk> Date: Tue, 16 Jun 2020 17:21:40 +0100 Subject: [PATCH 18/19] Remove incident listener inheritance --- .../src/TrackStateProvider.cpp | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index 9edbb12fa62..9e84373fdc2 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -12,8 +12,6 @@ // from Gaudi #include "GaudiAlg/GaudiTool.h" #include "GaudiKernel/DataObjectHandle.h" -#include "GaudiKernel/IIncidentListener.h" -#include "GaudiKernel/IIncidentSvc.h" #include "GaudiKernel/ToolHandle.h" // from TrackInterfaces @@ -153,7 +151,7 @@ namespace { } // namespace -class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider, IIncidentListener> { +class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider> { public: /// Standard constructor @@ -190,9 +188,6 @@ public: /// Clear the cache for a particular track void clearCache( const LHCb::Track& track ) const override; - /// incident service handle - void handle( const Incident& incident ) override; - private: StatusCode computeState( const TrackCache& tc, double z, LHCb::State& state, const LHCb::TrackTraj::StateContainer::const_iterator& position ) const; @@ -261,16 +256,6 @@ StatusCode TrackStateProvider::finalize() { return GaudiTool::finalize(); } -//============================================================================= -// Incident handle -//============================================================================= - -void TrackStateProvider::handle( const Incident& /* incident */ ) { - // only one incident type subscribed to, so no need to check type - // if ( IncidentType::BeginEvent == incident.type() ) - clearCache(); -} - //============================================================================= // Clear cache (and update some counters) //============================================================================= -- GitLab From edbddd80652369219f6fcc4c2af96b425d881116 Mon Sep 17 00:00:00 2001 From: Wouter Hulsbergen <wouter.hulsbergen@nikhef.nl> Date: Tue, 23 Jun 2020 11:03:59 +0200 Subject: [PATCH 19/19] update to make trackstateprovider thread safe --- .../src/TrackStateProvider.cpp | 130 ++++++++---------- .../TrackInterfaces/ITrackStateProvider.h | 8 +- 2 files changed, 59 insertions(+), 79 deletions(-) diff --git a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp index 099130d4b1a..0163b094ae2 100644 --- a/Tr/TrackExtrapolators/src/TrackStateProvider.cpp +++ b/Tr/TrackExtrapolators/src/TrackStateProvider.cpp @@ -1,5 +1,5 @@ /*****************************************************************************\ -* (c) Copyright 2000-2020 CERN for the benefit of the LHCb Collaboration * +* (c) Copyright 2000-2018 CERN for the benefit of the LHCb Collaboration * * * * This software is distributed under the terms of the GNU General Public * * Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * @@ -11,7 +11,6 @@ // from Gaudi #include "GaudiAlg/GaudiTool.h" -#include "GaudiKernel/DataObjectHandle.h" #include "GaudiKernel/ToolHandle.h" // from TrackInterfaces @@ -32,10 +31,9 @@ // STL #include <cstdint> +#include <deque> #include <memory> -#include <mutex> #include <numeric> -#include <thread> #include <unordered_map> // boost @@ -62,25 +60,28 @@ namespace { /// Create the key for a given track inline TkCacheKey trackID( const LHCb::Track& track ) { - // key is based on track memory addresses - TkCacheKey id = TkCacheKey( &track ); - boost::hash_combine( id, track.fitResult() ); - if ( !track.lhcbIDs().empty() ) { - boost::hash_combine( id, track.lhcbIDs().front().lhcbID() ); - boost::hash_combine( id, track.lhcbIDs().back().lhcbID() ); - } - return id; + TkCacheKey tkid = TkCacheKey( &track ); + if ( track.fitResult() ) { boost::hash_combine( tkid, track.fitResult() ); } + if ( track.parent() ) { boost::hash_combine( tkid, track.parent() ); } + // How many IDs are needed to 'guarantee' uniqueness in hash ?? + for ( const auto id : track.lhcbIDs() ) { boost::hash_combine( tkid, id.lhcbID() ); } + // if ( LIKELY( !track.lhcbIDs().empty() ) ) { + // boost::hash_combine( tkid, track.lhcbIDs().front().lhcbID() ); + // boost::hash_combine( tkid, track.lhcbIDs().back().lhcbID() ); + //} + return tkid; } // The TrackCache is basically just an 'extendable' TrackTraj class TrackCache final : public LHCb::TrackTraj { private: - const LHCb::Track* m_track{nullptr}; + const LHCb::Track* m_track = nullptr; std::deque<LHCb::State> m_ownedstates; double m_zFirstMeasurement{-9999}; public: + /// Constructor from a track TrackCache( const LHCb::Track& track ) : LHCb::TrackTraj( track ), m_track( &track ) { const auto state = track.stateAt( LHCb::State::Location::FirstMeasurement ); m_zFirstMeasurement = ( state ? state->z() : -9999 ); @@ -108,13 +109,13 @@ namespace { const LHCb::State* TrackCache::insertState( std::ptrdiff_t pos, LHCb::State&& o_state ) { // take ownership of state - m_ownedstates.push_back( o_state ); - auto state = &( m_ownedstates.back() ); + auto& state = m_ownedstates.emplace_back( std::move( o_state ) ); // get the vector with states auto& refstates = refStates(); - refstates.insert( std::next( refstates.begin(), pos ), state ); + // insert state + refstates.insert( std::next( refstates.begin(), pos ), &state ); assert( std::is_sorted( refstates.begin(), refstates.end(), compareStateZ ) ); // update the range of the trajectory @@ -123,12 +124,9 @@ namespace { // invalidate the cache TrackTraj::invalidateCache(); - return state; + return &state; } - /// Type for container of track caches - using TrackCaches = std::unordered_map<TkCacheKey, TrackCache>; - } // namespace class TrackStateProvider : public extends<GaudiTool, ITrackStateProvider> { @@ -137,12 +135,6 @@ public: /// Standard constructor TrackStateProvider( const std::string& type, const std::string& name, const IInterface* parent ); - /// initialize - StatusCode initialize() override; - - /// initialize - StatusCode finalize() override; - /// Compute the state of the track at position z. The third /// argument is the tolerance: if an existing state is found within /// a z-distance 'tolerance', that state is returned. @@ -168,6 +160,10 @@ public: /// Clear the cache for a particular track void clearCache( const LHCb::Track& track ) const override; +private: + /// Type for cache + using TrackCaches = std::unordered_map<TkCacheKey, TrackCache>; + private: StatusCode computeState( const TrackCache& tc, // const double z, // @@ -181,21 +177,30 @@ private: /// Get the track cache from the event store TrackCaches& trackcache() const { - auto* obj = m_caches.getIfExists(); - return const_cast<TrackCaches&>(obj ? *obj : *(m_caches.put( TrackCaches{} ))) ; + // auto* obj = m_caches.getIfExists(); + // return const_cast<TrackCaches&>( obj ? *obj : *( m_caches.put( TrackCaches{} ) ) ); + static constexpr auto loc = "TrackStateProviderCache"; + using CacheTES = AnyDataWrapper<TrackCaches>; + auto d = getIfExists<CacheTES>( loc ); + if ( UNLIKELY( !d ) ) { + d = new CacheTES( TrackCaches{} ); + put( d, loc ); + } + return d->getData(); } /// Create a cache entry TrackCache createCacheEntry( const TkCacheKey key, const LHCb::Track& track ) const; - /// Retrieve a single cache entry + /// Retrieve a cache entry TrackCache& cache( const LHCb::Track& track ) const; private: - mutable std::mutex m_mutex; - mutable DataObjectHandle<AnyDataWrapper<TrackCaches>> m_caches{this, Gaudi::DataHandle::Writer, "CacheLocation","TrackStateProviderCache"}; - ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; - ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; + // mutable DataObjectHandle<AnyDataWrapper<TrackCaches>> m_caches{this, Gaudi::DataHandle::Writer, "CacheLocation", + // "TrackStateProviderCache"}; + + ToolHandle<ITrackExtrapolator> m_extrapolator{"TrackMasterExtrapolator", this}; + ToolHandle<ITrackInterpolator> m_interpolator{"TrackInterpolator", this}; Gaudi::Property<bool> m_applyMaterialCorrections{this, "ApplyMaterialCorrections", true}; Gaudi::Property<double> m_linearPropagationTolerance{this, "LinearPropagationTolerance", 1.0 * Gaudi::Units::mm}; @@ -218,48 +223,22 @@ TrackStateProvider::TrackStateProvider( const std::string& type, // } //============================================================================= -// Initialization +// Clear cache //============================================================================= -StatusCode TrackStateProvider::initialize() { - StatusCode sc = GaudiTool::initialize(); - // retrieve tools - if ( sc.isSuccess() ) sc = m_extrapolator.retrieve(); - if ( sc.isSuccess() ) sc = m_interpolator.retrieve(); - return sc; -} - -//============================================================================= -// Finalize -//============================================================================= -StatusCode TrackStateProvider::finalize() { - m_extrapolator.release().ignore(); - m_interpolator.release().ignore(); - return GaudiTool::finalize(); -} - -//============================================================================= -// Clear cache (and update some counters) -//============================================================================= - void TrackStateProvider::clearCache() const { - auto& m_trackcache = trackcache(); - if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache. Size is " << m_trackcache.size() << "." << endmsg; - counter( "Number of tracks seen" ) += m_trackcache.size(); - counter( "Number of states added" ) += - std::accumulate( m_trackcache.begin(), m_trackcache.end(), 0, - []( auto n, const auto& p ) { return n + p.second.numOwnedStates(); } ); - m_trackcache.clear(); + auto& tc = trackcache(); + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Clearing cache. Size is " << tc.size() << "." << endmsg; } + tc.clear(); } //============================================================================= // Clear cache for a given track //============================================================================= - void TrackStateProvider::clearCache( const LHCb::Track& track ) const { - if ( msgLevel( MSG::DEBUG ) ) debug() << "Clearing cache for track: key=" << track.key() << endmsg; - auto& m_trackcache = trackcache(); - auto it = m_trackcache.find( trackID( track ) ); - if ( it != m_trackcache.end() ) m_trackcache.erase( it ); + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Clearing cache for track: key=" << track.key() << endmsg; } + auto& tc = trackcache(); + auto it = tc.find( trackID( track ) ); + if ( it != tc.end() ) { tc.erase( it ); } } //============================================================================= @@ -270,6 +249,7 @@ StatusCode TrackStateProvider::state( LHCb::State& state, // const LHCb::Track& track, // double z, // double ztolerance ) const { + auto& tc = cache( track ); // locate the closest state with lower_bound, but cache the @@ -316,7 +296,7 @@ StatusCode TrackStateProvider::state( LHCb::State& state, // const LHCb::State* TrackStateProvider::addState( TrackCache& tc, double z, LHCb::State::Location loc, std::ptrdiff_t position ) const { - LHCb::State state{loc}; + auto state = LHCb::State{loc}; state.setZ( z ); if ( position < 0 ) { position = std::distance( tc.states().cbegin(), @@ -381,6 +361,7 @@ StatusCode TrackStateProvider::computeState( const TrackCache& tc, // //============================================================================= TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Track& track ) const { + if ( msgLevel( MSG::DEBUG ) ) { debug() << "Creating track cache for track: key=" << track.key() << " hashID=" << key << " " << track.states().size() << endmsg; @@ -390,7 +371,7 @@ TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Tra TrackCache tc{track}; // make sure downstream tracks have a few ref states before the first measurement. - if ( track.type() == LHCb::Track::Downstream ) { + if ( track.type() == LHCb::Track::Types::Downstream ) { for ( const auto& loc : {std::pair{LHCb::State::Location::EndRich1, StateParameters::ZEndRich1}, std::pair{LHCb::State::Location::BegRich1, StateParameters::ZBegRich1}, std::pair{LHCb::State::Location::EndVelo, StateParameters::ZEndVelo}} ) { @@ -434,11 +415,12 @@ TrackCache TrackStateProvider::createCacheEntry( TkCacheKey key, const LHCb::Tra // retrieve TrackCache entry for a given track (from the cache) //============================================================================= TrackCache& TrackStateProvider::cache( const LHCb::Track& track ) const { - auto key = trackID( track ); - auto& m_trackcache = trackcache(); - auto it = m_trackcache.find( key ); - if ( it == m_trackcache.end() ) { - auto ret = m_trackcache.emplace( key, createCacheEntry( key, track ) ); + // get the cache from the stack + const auto key = trackID( track ); + auto& tc = trackcache(); + auto it = tc.find( key ); + if ( it == tc.end() ) { + auto ret = tc.emplace( key, createCacheEntry( key, track ) ); it = ret.first; } return it->second; diff --git a/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h b/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h index 59bca3ab338..2e9e3cbacaf 100644 --- a/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h +++ b/Tr/TrackInterfaces/TrackInterfaces/ITrackStateProvider.h @@ -1,5 +1,5 @@ /*****************************************************************************\ -* (c) Copyright 2000-2019 CERN for the benefit of the LHCb Collaboration * +* (c) Copyright 2000-2018 CERN for the benefit of the LHCb Collaboration * * * * This software is distributed under the terms of the GNU General Public * * Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * @@ -8,18 +8,17 @@ * granted to it by virtue of its status as an Intergovernmental Organization * * or submit itself to any jurisdiction. * \*****************************************************************************/ -#ifndef TRACKINTERFACES_ITRACKSTATEPROVIDER_H -#define TRACKINTERFACES_ITRACKSTATEPROVIDER_H 1 +#pragma once // Include files // ------------- // from Gaudi +#include "Event/Track.h" #include "Event/TrackParameters.h" #include "GaudiKernel/IAlgTool.h" // Forward declarations namespace LHCb { - class Track; class State; class TrackTraj; } // namespace LHCb @@ -58,4 +57,3 @@ struct ITrackStateProvider : extend_interfaces<IAlgTool> { /// Clear the cache for a particular track virtual void clearCache( const LHCb::Track& track ) const = 0; }; -#endif // TRACKINTERFACES_ITRACKSTATEPROVIDER_H -- GitLab