Bug #7052
Bug #6394: Verify that association query objects are created outside tight loops
FindManyP() usage in AggregateVertex, VertexCheater, TrackCheater and EventMaker modules
100%
Description
AggregateVertex
, VertexCheater
, TrackCheater
and EventMaker
modules show non-optimal usage of associations:
larreco/VertexFinder/AggregateVertex_module.cc
larreco/VertexFinder/VertexCheater_module.cc
larreco/TrackFinder/TrackCheater_module.cc
larreco/EventFinder/EventMaker_module.cc
I notice dubious practises in AggregateVertex::MatchV2T()
, VertexCheater::produce()
, TrackCheater::produce()
and EventMaker::produce()
.
The actions I recommend:
- since you use it, #include "art/Framework/Core/FindManyP.h"
- bring the queries FindManyP()
out of the loops
- use const std::vector<art::Ptr<T>>&
to store the result of FindManyP::at()
, that returns a constant reference
- in TrackCheater::produce()
, directly using a reference for hits
is not possible; I'd recommend using a pointer pHits
and after the value of the pointer is set, define a reference const std::vector<...>& hits = *pHits;
; or, alternatively, use a temporary const vector reference inside the loop and create hits (also const vector reference) as fmh.at(p)
, of course declaring p
in the scope containing the loop; this potentially avoids a number of copies
Contact person: Brian Rebel (brebel@fnal.gov)
History
#1 Updated by Brian Rebel over 6 years ago
- Status changed from Assigned to Resolved
- % Done changed from 0 to 100
Issues fixed by moving the FindMany(P) construction outside of loops. Accomplish same functionality by keeping track of the index of each object in the collection used to create the FindMany(P) when running the module algorithms.
#2 Updated by Gianluca Petrillo about 6 years ago
- Status changed from Resolved to Closed