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

Swap with empty vectors to free up memory in ElectronSeedProducer #9194

Merged
merged 1 commit into from May 21, 2015

Conversation

mark-grimes
Copy link

Using clear on temporary vectors wasn't freeing up the memory. Changed to a swap with an empty vector instead.

At 200 pileup this frees up about 40 Mb. Retained memory before (including the product, although that is a tiny fraction of it for this):
electronseedretained_before

And after applying this pull request:
electronseedretained_after

@cmsbuild
Copy link
Contributor

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

Swap with empty vectors to free up memory in ElectronSeedProducer

It involves the following packages:

RecoEgamma/EgammaElectronAlgos

@cmsbuild, @cvuosalo, @nclopezo, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @lgray 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.

@mark-grimes
Copy link
Author

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@mark-grimes
Copy link
Author

merge

cmsbuild added a commit that referenced this pull request May 21, 2015
Swap with empty vectors to free up memory in ElectronSeedProducer
@cmsbuild cmsbuild merged commit a84edd8 into cms-sw:CMSSW_6_2_X_SLHC May 21, 2015
@mark-grimes mark-grimes deleted the fixElectronSeedMemory branch May 21, 2015 07:18
@lgray
Copy link
Contributor

lgray commented May 21, 2015

Size of effect?

-L

On Thu, May 21, 2015 at 9:17 AM, cmsbuild notifications@github.com wrote:

Merged #9194 #9194.


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

@mark-grimes
Copy link
Author

About 40 Mb. For RSS and VmSize I was going to see what else I can squeeze out before doing a test - how I measure the memory individually by module inflates memory so I can't measure both at the same time.

Looking at what's next on the hitlist, PFClusterProducer has about 100 Mb retained but half of that is from beginLumi, which is probably a service or something. FastJetProducer has ~70 Mb and SeedGeneratorFromRegionHitsEDProducer has ~50 Mb. So that's maybe another 170 Mb I can squeeze out, then I'll do a run checking the RSS and VmSize.
Current state of the retained memory by class excluding product size at 200 pileup (top 25 only):
retainedmemory_nextsteps

@lgray
Copy link
Contributor

lgray commented May 21, 2015

Hi Mark,

The retained memory in PFClusterProducer is either the product being made
or the HGC topology classes (but those are pretty light iirc).

I'd guess that major gains can be made in the fastjet producer. Jets are
fairly hefty data objects and there is lots of translation back and forth
between fastjet objects and CMSSW, so there's likely some cache that is not
emptied once done. There are also a huge number of jets produced per event.

I'll take a look a PFEGammaProducer (here it's the old version) to see if
anything can be better managed.

Best,
-Lindsey

On Thu, May 21, 2015 at 9:55 AM, Mark Grimes notifications@github.com
wrote:

About 40 Mb. For RSS and VmSize I was going to see what else I can squeeze
out before doing a test - how I measure the memory individually by module
inflates memory so I can't measure both at the same time.

Looking at what's next on the hitlist, PFClusterProducer has about 100 Mb
retained but half of that is from beginLumi, which is probably a service or
something. FastJetProducer has ~70 Mb and
SeedGeneratorFromRegionHitsEDProducer has ~50 Mb. So that's maybe another
170 Mb I can squeeze out, then I'll do a run checking the RSS and VmSize.
Current state of the retained memory by class excluding product size at
200 pileup (top 25 only):
[image: retainedmemory_nextsteps]
https://cloud.githubusercontent.com/assets/6480160/7743843/42dddd1c-ff96-11e4-92b5-8a189942f596.png


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

@mark-grimes
Copy link
Author

The plot is excluding the memory used by the products. If the products are included:
retainedmemoryincludingproducts_nextsteps

I think the HGC topology classes are the big chunk in beginLumi (about 50Mb). Looking at the split by instance, this is lumped in with particleFlowClusterHGCEE since it's the first run. particleFlowClusterHGCHEF shares the same instances (right?) so doesn't have the same jump in beginLumi. Discounting that 50 Mb they retain about 25 Mb each which isn't the produced collections.
pfcluster_retained

I think I found the cache in FastjetJetProducer almost straight away, I'll test to see if clearing it gives back the ~70Mb. There are 24 instances of the class running so this one change should hopefully bring a big gain.

@lgray
Copy link
Contributor

lgray commented May 21, 2015

Hi Mark,

OK thanks for the clarification!

Good that the jet producers seemed easy. We should port this to the Run2
RECO if it's a big effect.

The geometry instances are shared but the topology may not be (the
infrastructure wasn't fully integrated).
Let me check what actually went in here.

Best,
-Lindsey

On Thu, May 21, 2015 at 10:30 AM, Mark Grimes notifications@github.com
wrote:

The plot is excluding the memory used by the products. If the products are
included:
[image: retainedmemoryincludingproducts_nextsteps]
https://cloud.githubusercontent.com/assets/6480160/7744489/9f4f7f10-ff9b-11e4-9143-be0004b3c6a8.png

I think the HGC topology classes are the big chunk in beginLumi (about
50Mb). Looking at the split by instance, this is lumped in with
particleFlowClusterHGCEE since it's the first run.
particleFlowClusterHGCHEF shares the same instances (right?) so doesn't
have the same jump in beginLumi. Discounting that 50 Mb they retain about
25 Mb each which isn't the produced collections.
[image: pfcluster_retained]
https://cloud.githubusercontent.com/assets/6480160/7744379/9f720c02-ff9a-11e4-8be5-c9b43ccebc87.png

I think I found the cache in FastjetJetProducer almost straight away, I'll
test to see if clearing it gives back the ~70Mb. There are 24 instances of
the class running so this one change should hopefully bring a big gain.


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

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

3 participants