Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SiPixelPhase1TrackingParticleV does not clear trackIdToHitPtr_ #21079

Closed
Dr15Jones opened this issue Oct 30, 2017 · 12 comments
Closed

SiPixelPhase1TrackingParticleV does not clear trackIdToHitPtr_ #21079

Dr15Jones opened this issue Oct 30, 2017 · 12 comments

Comments

@Dr15Jones
Copy link
Contributor

SiPixelPhase1TrackingParticleV has the member data

std::vector<std::pair<unsigned int, const PSimHit *>> trackIdToHitPtr_

which it fills with PSimHits from the event and then does a lookup later for the hits. The problem is the code never clears the container. This means the container holds stale pointers which were from old events.

The relevant function is:

void SiPixelPhase1TrackingParticleV::analyze(const edm::Event& iEvent, const edm::EventSetup& iSetup) {

The simplest solution would be to remove trackIdToHitPtr_ as a member data and just have it a local variable of the module.

@cmsbuild
Copy link
Contributor

A new Issue was created by @Dr15Jones Chris Jones.

@davidlange6, @Dr15Jones, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@Dr15Jones
Copy link
Contributor Author

@slava77 here is the memory problem.

@Dr15Jones
Copy link
Contributor Author

@davidcarbonis

@Dr15Jones
Copy link
Contributor Author

assign dqm

@cmsbuild
Copy link
Contributor

New categories assigned: dqm

@kmaeshima,@vanbesien,@vazzolini,@dmitrijus you have been requested to review this Pull request/Issue and eventually sign? Thanks

@davidcarbonis
Copy link
Contributor

@Dr15Jones Thanks for spotting the cause of this. Will resolve shortly.

@davidcarbonis
Copy link
Contributor

PR #21084 opened with fix.

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 2, 2017

This issue is fully signed and ready to be closed.

@Dr15Jones
Copy link
Contributor Author

Fixed

@davidcarbonis
Copy link
Contributor

@Dr15Jones I note that the current target for the closed PR is CMSSW_10_0_X and that there isn't a 9_4_0 release (only pre-release) yet. Would it be prudent/worth it to open a PR for adding this fix to 9_4_0 (assuming that the plans for using 9_4_0 haven't changed)?

@Dr15Jones
Copy link
Contributor Author

@davidlange6 would you want a 9_4 backport of #21084 ? It would get rid of some memory hoarding and fix a reproducibility issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants