Project

General

Profile

Bug #7052

Bug #6394: Verify that association query objects are created outside tight loops

FindManyP() usage in AggregateVertex, VertexCheater, TrackCheater and EventMaker modules

Added by Gianluca Petrillo over 6 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Reconstruction
Target version:
-
Start date:
09/22/2014
Due date:
% Done:

100%

Estimated time:
1.00 h
Occurs In:
Experiment:
LArSoft
Co-Assignees:
Duration:

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 ()

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

Also available in: Atom PDF