memory leak fixes

[ptraczyk]

fixes requested in slava77#22

[cmsbuild]

A new Pull Request was created by @ptraczyk (Piotr Traczyk) for CMSSW_7_0_X.

memory leak fixes

It involves the following packages:

RecoLocalMuon/DTSegment

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[Dr15Jones]

Why not just use a std::vector<std::shared_ptr<DTHitPairForFit>> and then you never have to think about memory management?

[slava77]

Chris, I agree, this is a better choice.
I'd stick with what we have in this PR.
Piotr, please make changes to use the smart pointers in the 71X PR with new DT features.

[ptraczyk]

yes, I need to make (simple) changes in a few places for this to work. I'll
leave this as it is here and change the 71X code

On Wed, Feb 5, 2014 at 5:01 PM, slava77 [email protected] wrote:

In RecoLocalMuon/DTSegment/src/DTMeantimerPatternReco.cc:

@@ -77,6 +77,9 @@
delete *(cand++); // delete the candidate!
}

• for (vector<DTHitPairForFit*>::iterator it = hitsForFit.begin(), ed = hitsForFit.end();

Chris, I agree, this is a better choice.
I'd stick with what we have in this PR.
Piotr, please make changes to use the smart pointers in the 71X PR with
new DT features.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2221/files#r9465354
.

[cmsbuild]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2221/15/summary.html

[slava77]

@Degano @nclopezo
Alessandro and David, could you please rerun the jenkins tests, hoping this time they will be reproducible. The last run had unexpected changes, e.g. in tracking plots in 25.0

[nclopezo]

Hi Slava,

I just started them again.

[nclopezo]

@slava77
Hi Slava,

you should be able to see them now

[ktf]

David, I still cannot see:

https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_7_0_X_2014-01-29-0200+2221/1093/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/comparisonResults/Btag__SET_GLOBAL_CenJet_dR.png

[slava77]

These are due to problems with SET_GLOBAL monitoring
@deguio Federico, could you please follow up.
There are some NaNs in the plots, which breaks the comparison/diff logic.

[cmsbuild]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2221/18/summary.html

[slava77]

+1
tested #2221 492a05f
in CMSSW_7_0_X_2014-02-03-0200/sign303

no differences in the outputs, running the short matrix and few K events in 10 GeV and 1 TeV gun samples.

Valgrind shows that the leaks in DTRecSegment4DProducer::produce are gone

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_0_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

[slava77]

@ktf @davidlange6
Giulio and David: what should I expect to happen here?
One option is that this gets merged to 71X and shows up happily in at least one IB there and then gets merged to 70X.
Thoughts?

One observation here is that I don't see changes in the total memory footprint of the job based on RSS or VSIZE reported in the end of the path by the memory check.
Here, I checked a fastsim 5.2 (10 GeV muons) modified to generate 40 muons and running 1K events.
So, while the leak is clearly there, that change in memory probably never gets visible to the memory checker.

[ktf]

I like your idea. Since 71X is based on 70X, this should merge by simply opening a different pull request, using the same branch.

[slava77]

Giulio, would you like me to make it, or would you rather do it yourself? Please let me know

[ktf]

#2340.

[davidlange6]

+1

[nclopezo]

Hi all,

Given that #2340 was merged last Friday I would like to confirm if it is ok to merge this pull request now.

[davidlange6]

David -
I will go through this and any other 70x items tomorrow as input to the ORP.

david

On Feb 10, 2014, at 1:42 PM, David Mendez [email protected]
wrote:

Hi all,

Given that #2340 was merged last Friday I would like to confirm if it is ok to merge this pull request now.

—
Reply to this email directly or view it on GitHub.