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 to 74X) #9210

Merged
merged 3 commits into from
Jun 10, 2015

Conversation

mark-grimes
Copy link

This forward ports some memory improvements in FastjetJetProducer from 6_2_X_SLHC; there are some plots on #9203 with the memory saving. The 75X port is #9206, I was also advised to port to 74X.
This bit below is just the description copied from the 75X PR:

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.

@cmsbuild
Copy link
Contributor

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

Trim FastjetJetProducer memory (forward port #9203 to 74X)

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.
@Degano 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

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs unless changes or unless it breaks tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

@slava77
Copy link
Contributor

slava77 commented May 23, 2015

On 5/23/15 1:58 PM, Carl Vuosalo wrote:

Memory usage seems to go down very slightly

30-50 MB reduction is a lot, actually

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @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.

5 participants