Trim FastjetJetProducer memory

[mark-grimes]

FastjetJetProducer keeps a chunk of memory between events unnecessarily. With the 24 instances in the standard sequence this amounts to more than 70 Mb at 200 pileup.
Here is the retained memory, excluding produced collection sizes, summed over all instances of the class at 200 pileup:

Here is the same thing after applying this pull request:

Swapping out the work vectors with empty vectors saves about 10 Mb of that 70. The remainder is from deleting the fjClusterSeq_ object in between events (it's created new each event anyway).

@lgray

[mark-grimes]

please test

[cmsbuild]

The tests are being triggered in jenkins.

[cmsbuild]

A new Pull Request was created by @mark-grimes (Mark Grimes) for CMSSW_6_2_X_SLHC.

Trim FastjetJetProducer memory

It involves the following packages:

RecoJets/JetProducers

@cmsbuild, @cvuosalo, @nclopezo, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @jdolen, @ahinzmann, @TaiSakuma, @yslai, @nhanvtran, @schoef, @mariadalfonso this is something you requested to watch as well.
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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@fratnikov, @mark-grimes you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[lgray]

nice!

On Thu, May 21, 2015 at 3:24 PM, Mark Grimes notifications@github.com
wrote:

FastjetJetProducer keeps a chunk of memory between events unnecessarily.
With the 24 instances in the standard sequence this amounts to more than 70
Mb at 200 pileup.
Here is the retained memory, excluding produced collection sizes, summed
over all instances of the class at 200 pileup:
[image: fastjet_before]
https://cloud.githubusercontent.com/assets/6480160/7749331/9f302cae-ffc4-11e4-8707-1f0e68eaaf3c.png

Here is the same thing after applying this pull request:
[image: fastjet_after]
https://cloud.githubusercontent.com/assets/6480160/7749346/bc0104c0-ffc4-11e4-8e70-d5af24658b99.png

Swapping out the work vectors with empty vectors saves about 10 Mb of that
70. The remainder is from deleting the fjClusterSeq_ object in between
events (it's created new each event anyway).

@lgray https://github.com/lgray

You can view, comment on, or merge this pull request online at:

#9203
Commit Summary

• Swap the work vectors out to empty temps to free up memory between
  events
• Free the Fastjet ClusterSequence between events to save memory

File Changes

• M RecoJets/JetProducers/plugins/FastjetJetProducer.cc
  https://github.com/cms-sw/cmssw/pull/9203/files#diff-0 (11)
• M RecoJets/JetProducers/plugins/VirtualJetProducer.cc
  https://github.com/cms-sw/cmssw/pull/9203/files#diff-1 (7)

Patch Links:

• https://github.com/cms-sw/cmssw/pull/9203.patch
• https://github.com/cms-sw/cmssw/pull/9203.diff

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

[rappoccio]

This looks good, could we forward-port this also to the other active branches?

[mark-grimes]

@rappoccio, sure although I assume the gains scale with pileup, so the difference won't be as impressive as here. Just 75X or do you want any other branches?

[rappoccio]

@mark-grimes We can only do 75x at this point, I think. Thanks a lot for the nice catch!

[cmsbuild]

+1
Tested at: d832b38
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9203/5290/summary.html

[lgray]

@rappoccio @mark-grimes 75X and 74X are open for fixes like this, I am pretty sure.

@slava77 comment?

[lgray]

@mark-grimes Also, I have looked at the EGAlgo, looks pretty straightforward too. I'll send you a PR soon.

[lgray]

@mark-grimes Just sent you a PR to this PR.

[lgray]

@mark-grimes looking at PFClusterProducer now. Can you post a break down per module instance of the non-product retained memory?

[mark-grimes]

@rappoccio, 75X PR is #9206. Let me know if you want a 74X port as well.

@lgray, that's the lower plot on #9194 (comment), i.e. https://cloud.githubusercontent.com/assets/6480160/7744379/9f720c02-ff9a-11e4-8be5-c9b43ccebc87.png. Oddly everything other than the HGCal instances has next to zero, which is a much bigger difference than I'd expect from the increased collection size (the plot doesn't include product size but work vectors usually scale with collection size). Maybe it is the topology, although I'd expect that not to fluctuate event to event.

[lgray]

HGCClusterizer doesn't use the topology, so the answer is somewhere a bit
deeper.
Anyway, I am filling up the PR to this PR with cache wipes.
Hold a moment before merging. I'll post when ready.

-L

On Thu, May 21, 2015 at 5:07 PM, Mark Grimes notifications@github.com
wrote:

@rappoccio https://github.com/rappoccio, 75X PR is #9206
#9206. Let me know if you want a
74X port as well.

@lgray https://github.com/lgray, that's the lower plot on #9194
(comment)
#9194 (comment), i.e.
https://cloud.githubusercontent.com/assets/6480160/7744379/9f720c02-ff9a-11e4-8be5-c9b43ccebc87.png.
Oddly everything other than the HGCal instances has next to zero, which is
a much bigger difference than I'd expect from the increased collection size
(the plot doesn't include product size but work vectors usually scale with
collection size). Maybe it is the topology, although I'd expect that not to
fluctuate event to event.

—
Reply to this email directly or view it on GitHub
#9203 (comment).

[slava77]

74X life time is expected to be short.
If it's not much effort, please make a PR there as well.

On 5/21/15 10:07 AM, Mark Grimes wrote:

@rappoccio https://github.com/rappoccio, 75X PR is #9206
#9206. Let me know if you want a
74X port as well.

@lgray https://github.com/lgray, that's the lower plot on #9194
(comment)
#9194 (comment), i.e.
https://cloud.githubusercontent.com/assets/6480160/7744379/9f720c02-ff9a-11e4-8be5-c9b43ccebc87.png.
Oddly everything other than the HGCal instances has next to zero, which
is a much bigger difference than I'd expect from the increased
collection size (the plot doesn't include product size but work vectors
usually scale with collection size). Maybe it is the topology, although
I'd expect that not to fluctuate event to event.

—
Reply to this email directly or view it on GitHub
#9203 (comment).

[lgray]

@mark-grimes Whenever you are ready please go ahead and merge mark-grimes#1 !

[mark-grimes]

merge