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

Trim FastjetJetProducer memory (forward port #9203) #9206

Merged
merged 3 commits into from May 24, 2015

Conversation

mark-grimes
Copy link

This forward ports some memory improvements found in 6_2_X_SLHC. At 200 pileup it saved ~70Mb memory, there are plots on #9203. Presumably at lower pileup the savings aren't as good.

Note that this is from empirically checking where the memory was being used - there could be other subclasses of VirtualJetProducer that would benefit from a similar fix. I didn't want to touch the fjClusterSeq_ object in the base class because I didn't know how the other subclasses use it - for FastjetJetProducer I can see that it's recreated anew with every event.

At 200 pileup 1/7th of the memory saving was from swapping out the work vectors, and 6/7th was from freeing the fjClusterSeq_ object.

@rappoccio, @lgray

@rappoccio
Copy link
Contributor

This is a good cleanup anyway, so I think it should be included in 75x also.

Cheers,
Sal

@cmsbuild
Copy link
Contributor

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

Trim FastjetJetProducer memory (forward port #9203)

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.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@slava77
Copy link
Contributor

slava77 commented May 21, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

+1

For #9206 927422c

Reduces memory usage by clearing some FastJetProducer objects after they are no longer needed. No change in monitored quantities should occur. #9210 (already approved) is the 74X version of this PR.

The code changes are satisfactory, as are the Jenkins tests. Tests with workflow 25202.0_TTbar_13 against baseline CMSSW_7_5_X_2015-05-20-2300 using 50 events show no significant differences. Similar tests for #9210 show the same. Memory usage seems to go down slightly:

CMSSW_7_5_X_2015-05-20-2300
Summary for 50 events (sizes in MB)
Max VSIZ 3740.59 on evt 33 ; max RSS 3099.75 on evt 28

#9206
Summary for 50 events
Max VSIZ 3719.09 on evt 33 ; max RSS 3086.58 on evt 28

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @nclopezo, @smuzaffar

@davidlange6
Copy link
Contributor

+1

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

Successfully merging this pull request may close these issues.

None yet

6 participants