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

Fix PFCluster and PFEGamma memory #9227

Merged
merged 3 commits into from May 22, 2015

Conversation

mark-grimes
Copy link

Memory fixes from Lindsey.

Retained memory excluding products before applying this:
lgray_before

After applying this pull request:
lgray_after

Note that the yellow line (ElectronSeedProducer) was fixed in #9194, but I didn't have a "before" plot with that fix. So a big saving of 40-50 Mb from PFClusterProducer. No visible change in PFEGammaProducer, although that could be washed out from the big allocation in beginRun. That's probably unavoidable from a geom service or something (though I haven't checked) - the way I count the memory, services get lumped in with the first module that calls them.

So current state (this is basically the "after" plot above, but not range limited to match the "before" and the new top 25 rather than the old top 25):
currentstate_retained
Now a lot of the big allocations are in beginRun or beginLumi, which as mentioned above I'm going to assume are unavoidable for now. Might not be but I'll check those if and when memory becomes a problem again.
Only big thing left that makes the allocations in the events now is SeedGeneratorFromRegionHitsProducer. I'll compare against 75X because I think Vincenzo made a lot of improvements. Once I get that down I'll run a check of VmSize and RSS and see where we stand in the big picture.

@lgray

@mark-grimes
Copy link
Author

please test

@lgray
Copy link
Contributor

lgray commented May 22, 2015

I'll poke at PFEGProducer a bit more... it is very odd that it takes so
much memory... PFAlgo uses the same MVAs and doesn't show up on this plot!

-Lindsey

On Fri, May 22, 2015 at 11:58 AM, Mark Grimes notifications@github.com
wrote:

please test


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

@cmsbuild cmsbuild added this to the Next CMSSW_6_2_X_SLHC milestone May 22, 2015
@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

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

Fix PFCluster and PFEGamma memory

It involves the following packages:

RecoParticleFlow/PFClusterProducer
RecoParticleFlow/PFProducer

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

Is PFAlgo a service or something? I can only break down by module (EDProducer/EDAnalyzer/etcetera) and there's nothing in the list of 580 running modules that matches the name PFAlgo in any way; or in the class name of the 203 different module classes.
If they use MVAs from a service or some other kind of singleton (basically, if they're using the same instances) it will just show up in the module that first allocates them.

@lgray
Copy link
Contributor

lgray commented May 22, 2015

Hi Mark,

It's not from a service, the file is read in separately using TMVA::Reader
in each Algorithm (which is owned by a producer).

-L

On Fri, May 22, 2015 at 12:13 PM, Mark Grimes notifications@github.com
wrote:

Is PFAlgo a service or something? I can only break down by module
(EDProducer/EDAnalyzer/etcetera) and there's nothing in the list of 580
running modules that matches the name PFAlgo in any way; or in the class
name of the 203 different module classes.
If they use MVAs from a service or some other kind of singleton
(basically, if they're using the same instances) it will just show up in
the module that first allocates them.


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

@mark-grimes
Copy link
Author

I'll hack at PFEGammaProducer and see which line that beginRun allocation comes in. My money is on a completely innocuous service call - could be a basic one used by loads of things but PFEGammaProducer is just the first to use it.

@mark-grimes
Copy link
Author

Okay, so hacking in some printouts of the memory, the memory after each line (using SLHC25_patch6 line numbers) is:

line 239 13.7829 MiB
line 246 13.7829 MiB
line 248 24.0963 MiB
line 250 34.3503 MiB
line 252 54.0583 MiB
line 254 73.778 MiB
line 256 93.4859 MiB
line 259 113.194 MiB
line 265 113.194 MiB
line 268 113.194 MiB

Blank lines are in there to get the memory at the start and end. So it's basically creating objects to hold the result of database records as far as I can see, which is unavoidable. Once another module asks for the same record I assume it gets a pointer to the already created object, hence it won't show up for anything else.

@lgray
Copy link
Contributor

lgray commented May 22, 2015

OK, thanks for taking a look.
Let me make sure all of these DB objects are really used in PFEGAlgo, it
may be the case they are actually not.
However, this would just shuffle things between PFEGAlgo and PFAlgo, so
probably not worth it really.

-L

On Fri, May 22, 2015 at 1:16 PM, Mark Grimes notifications@github.com
wrote:

Okay, so hacking in some printouts of the memory, the memory after each
line (using SLHC25_patch6 line numbers) is:

line 239
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L239
13.7829 MiB
line 246
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L246
13.7829 MiB
line 248
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L248
24.0963 MiB
line 250
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L250
34.3503 MiB
line 252
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L252
54.0583 MiB
line 254
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L254
73.778 MiB
line 256
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L256
93.4859 MiB
line 259
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L259
113.194 MiB
line 265
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L265
113.194 MiB
line 268
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_0_SLHC25_patch6/RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc#L268
113.194 MiB

Blank lines are in there to get the memory at the start and end. So it's
basically creating objects to hold the result of database records as far as
I can see, which is unavoidable. Once another module asks for the same
record I assume it gets a pointer to the already created object, hence it
won't show up for anything else.


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

@cmsbuild
Copy link
Contributor

@mark-grimes
Copy link
Author

merge

cmsbuild added a commit that referenced this pull request May 22, 2015
Fix PFCluster and PFEGamma memory
@cmsbuild cmsbuild merged commit 28137cf into cms-sw:CMSSW_6_2_X_SLHC May 22, 2015
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