From e4e028cec997ef0042b2854f6192ca4078154deb Mon Sep 17 00:00:00 2001 From: Matevz Tadel Date: Mon, 30 Aug 2021 11:57:56 -0700 Subject: [PATCH 1/2] Use the first hit added in case of overlap in BkFitFitTracks, as is done in BkFitFitTracksBH. --- mkFit/MkFinder.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/mkFit/MkFinder.cc b/mkFit/MkFinder.cc index e31a39eb..72ba1324 100644 --- a/mkFit/MkFinder.cc +++ b/mkFit/MkFinder.cc @@ -1488,10 +1488,14 @@ void MkFinder::BkFitFitTracks(const EventOfHits & eventofhits, if (CurNode[i] >= 0 && HoTNodeArr[ i ][ CurNode[i] ].m_hot.layer == layer) { - // XXXX - this is now different - overlap info has not yet been exported - - // XXXX Note if I remove this hit (i.e., m_hot.index = -1), the overlap might stay. - // Need to account for this in exportTrack(). + // Skip the overlap hits -- if they exist. + // 1. Overlap hit gets placed *after* the original hit in TrackCand::exportTrack() + // which is *before* in the reverse iteration that we are doing here. + // 2. Seed-hit merging can result in more than two hits per layer. + // while (CurHit[i] > 0 && HoTArr[ i ][ CurHit[i] - 1 ].layer == layer) --CurHit[i]; + while (HoTNodeArr[ i ][ CurNode[i] ].m_prev_idx >= 0 && + HoTNodeArr[ i ][ HoTNodeArr[ i ][ CurNode[i] ].m_prev_idx ].m_hot.layer == layer) + CurNode[i] = HoTNodeArr[ i ][ CurNode[i] ].m_prev_idx; const Hit &hit = L.GetHit( HoTNodeArr[ i ][ CurNode[i] ].m_hot.index ); From 1d7b0616f36beab73014931060c10621f7739fc8 Mon Sep 17 00:00:00 2001 From: Matevz Tadel Date: Thu, 2 Sep 2021 16:29:47 -0700 Subject: [PATCH 2/2] Use custom allocator for vector and preallocate memory for them in a single allocation. Jemalloc used in cmssw can return space from allocations done from different threads (it seems). --- mkFit/HitStructures.h | 115 ++++++++++++++++++++++++++++++++++++++---- mkFit/MkBuilder.cc | 2 +- mkFit/MkBuilder.h | 6 ++- 3 files changed, 111 insertions(+), 12 deletions(-) diff --git a/mkFit/HitStructures.h b/mkFit/HitStructures.h index 3c448a73..0da4649b 100644 --- a/mkFit/HitStructures.h +++ b/mkFit/HitStructures.h @@ -392,6 +392,69 @@ struct HitMatchPair } }; +template class CcPool +{ + std::vector m_mem; + std::size_t m_pos = 0; + std::size_t m_size = 0; + +public: + void reset(std::size_t size) + { + if (size > m_mem.size()) + m_mem.resize(size); + m_pos = 0; + m_size = size; + + // printf("CcP reset to %zu\n", size); + } + + CcPool(std::size_t size=0) + { + if (size) reset(size); + } + + T* allocate(std::size_t n) + { + if (m_pos + n > m_size) throw std::bad_alloc(); + T* ret = & m_mem[m_pos]; + m_pos += n; + // printf("CcP alloc %zu\n", n); + return ret; + } + + void deallocate(T *p, std::size_t n) noexcept + { + // we do not care, implied on reset(). + // printf("CcP dealloc %zu\n", n); + } +}; + +template class CcAlloc +{ + CcPool *m_pool; + +public: + typedef T value_type; + + CcAlloc(CcPool *p) : m_pool(p) {} + + const void* pool_id() const { return m_pool; } + + T* allocate(std::size_t n) + { + return m_pool->allocate(n); + } + + void deallocate(T *p, std::size_t n) noexcept + { + m_pool->deallocate(p, n); + } +}; + +template +bool operator==(const CcAlloc &a, const CcAlloc &b) { return a.pool_id() == b.pool_id(); } + //------------------------------------------------------------------------------ @@ -488,9 +551,11 @@ inline float getScoreCand(const TrackCand& cand1, bool penalizeTailMissHits=fals // This inheritance sucks but not doing it will require more changes. -class CombCandidate : public std::vector +class CombCandidate : public std::vector> { public: + using allocator_type = CcAlloc; + enum SeedState_e { Dormant = 0, Finding, Finished }; TrackCand m_best_short_cand; @@ -510,13 +575,31 @@ class CombCandidate : public std::vector std::vector m_overlap_hits; // XXXX HitMatchPair could be a member in TrackCand - CombCandidate() : + CombCandidate(const allocator_type& alloc) : + std::vector>(alloc), m_state(Dormant), m_pickup_layer(-1) {} - // Need this so resize of EventOfCombinedCandidates::m_candidates can reuse vectors used here. + // Required by std::uninitialized_fill_n when declaring vector in EventOfCombCandidates + CombCandidate(const CombCandidate& o) : + std::vector>(o), + m_state(o.m_state), + m_pickup_layer(o.m_pickup_layer), + m_lastHitIdx_before_bkwsearch(o.m_lastHitIdx_before_bkwsearch), + m_nInsideMinusOneHits_before_bkwsearch(o.m_nInsideMinusOneHits_before_bkwsearch), + m_nTailMinusOneHits_before_bkwsearch(o.m_nTailMinusOneHits_before_bkwsearch), +#ifdef DUMPHITWINDOW + m_seed_algo(o.m_seed_algo), + m_seed_label(o.m_seed_label), +#endif + m_hots_size(o.m_hots_size), + m_hots(o.m_hots), + m_overlap_hits(o.m_overlap_hits) + {} + + // Required for std::swap(). CombCandidate(CombCandidate&& o) : - std::vector(std::move(o)), + std::vector>(std::move(o)), m_best_short_cand(std::move(o.m_best_short_cand)), m_state(o.m_state), m_pickup_layer(o.m_pickup_layer), @@ -538,13 +621,13 @@ class CombCandidate : public std::vector // for (auto &tc : *this) tc.setCombCandidate(this); } - // Need this for std::swap when filtering EventOfCombinedCandidates::m_candidates. + // Required for std::swap when filtering EventOfCombinedCandidates::m_candidates. // We do not call clear() on vectors as this will be done via EoCCs reset. // Probably would be better (clearer) if there was a special function that does // the swap in here or in EoCCs. CombCandidate& operator=(CombCandidate&& o) { - std::vector::operator=( std::move(o) ); + std::vector>::operator=( std::move(o) ); m_best_short_cand = std::move(o.m_best_short_cand); m_state = o.m_state; m_pickup_layer = o.m_pickup_layer; @@ -566,8 +649,9 @@ class CombCandidate : public std::vector void Reset(int max_cands_per_seed, int expected_num_hots) { - reserve(max_cands_per_seed); // we should never exceed this - clear(); + std::vector> tmp(get_allocator()); + swap(tmp); + reserve(max_cands_per_seed); // we *must* never exceed this m_best_short_cand.setScore( getScoreWorstPossible() ); @@ -660,6 +744,8 @@ inline void TrackCand::addHitIdx(int hitIdx, int hitLyr, float chi2) class EventOfCombCandidates { + CcPool m_cc_pool; + public: std::vector m_candidates; @@ -668,6 +754,7 @@ class EventOfCombCandidates public: EventOfCombCandidates(int size=0) : + m_cc_pool (), m_candidates(), m_capacity (0), m_size (0) @@ -675,15 +762,23 @@ class EventOfCombCandidates void Reset(int new_capacity, int max_cands_per_seed, int expected_num_hots = 128) { + m_cc_pool.reset(new_capacity * max_cands_per_seed); if (new_capacity > m_capacity) { - m_candidates.resize(new_capacity); + CcAlloc alloc(&m_cc_pool); + std::vector tmp(new_capacity, alloc); + m_candidates.swap(tmp); m_capacity = new_capacity; } - for (int s = 0; s < m_capacity; ++s) + for (int s = 0; s < new_capacity; ++s) { m_candidates[s].Reset(max_cands_per_seed, expected_num_hots); } + for (int s = new_capacity; s < m_capacity; ++s) + { + m_candidates[s].Reset(0, 0); + } + m_size = 0; } diff --git a/mkFit/MkBuilder.cc b/mkFit/MkBuilder.cc index 2f8a1307..6aa15dae 100644 --- a/mkFit/MkBuilder.cc +++ b/mkFit/MkBuilder.cc @@ -1460,7 +1460,7 @@ void MkBuilder::find_tracks_load_seeds(const TrackVec &in_seeds) // m_tracks can be used for BkFit. m_tracks.clear(); - m_event_of_comb_cands.Reset((int) in_seeds.size(), m_job->params().maxCandsPerSeed); + m_event_of_comb_cands.Reset((int) in_seeds.size(), m_job->max_max_cands()); import_seeds(in_seeds, [&](const Track& seed, int region){ m_event_of_comb_cands.InsertSeed(seed, region); }); } diff --git a/mkFit/MkBuilder.h b/mkFit/MkBuilder.h index 5d602a6a..06d414e2 100644 --- a/mkFit/MkBuilder.h +++ b/mkFit/MkBuilder.h @@ -70,7 +70,11 @@ class MkJob const auto& steering_params(int i) { return m_iter_config.m_steering_params[i]; } - const auto& params() const { return m_iter_config.m_params; } + const auto& params() const { return m_iter_config.m_params; } + const auto& params_bks() const { return m_iter_config.m_backward_params; } + + int max_max_cands() const + { return std::max(params().maxCandsPerSeed, params_bks().maxCandsPerSeed); } const std::vector* get_mask_for_layer(int layer) {