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

reduction of in-module memory peaks (backport of #16375) #16374

Merged

Conversation

slava77
Copy link
Contributor

@slava77 slava77 commented Oct 27, 2016

Significant in-module memory use peaks were found in tests of HLTPhysicsIsolatedBunch data in 283171 (LS 60 was used in tests):

  • particleFlowDisplacedVertex with ~2GB: the blow up happened in AdaptiveVertexFitter trying to fit vertices with 3-4 K tracks passed to the algo without pre-filtering with a simpler fitting.
    • Apparently the pre-filtering was designed with a 1K track preselection in mind. In a normal situation we should not have displaced vertices with O(1000) tracks. So, the candidate selection will need to be updated.
    • main change in this PR is to skip candidates with more than 1000 tracks (in addition to some simpler cosmetics). This should be rare enough in Run2 that we can have this update in the 80X.
  • particleFlowBlock with up to ~1GB: here, map hash initialization was made too large
    • HO-to-others linking may need to be revisited to see if cases of linking 10K block elements (as happened in these events) is meaningful
    • the main change in this PR for PFBA is to limit the block link map reserve to 1M (this apparently rarely ever happens even in PU200 in simulation). The cost is some extra CPU to extend the hash if the links size in the map ever gets over the 1M count. I have also changed the indexing from 64 bit size_t to unsigned_int.

No changes are expected in jenkins tests.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @slava77 (Slava Krutelyov) for CMSSW_8_0_X.

It involves the following packages:

RecoParticleFlow/PFProducer
RecoParticleFlow/PFTracking

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@mmarionncern, @lgray, @rafaellopesdesa, @bachtis, @cbernet this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@slava77 slava77 changed the title reduction of in-module memory peaks reduction of in-module memory peaks (backport of #16375) Oct 27, 2016
@slava77
Copy link
Contributor Author

slava77 commented Oct 27, 2016

backport of #16375

@slava77
Copy link
Contributor Author

slava77 commented Oct 27, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 27, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16016/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor Author

slava77 commented Oct 29, 2016

+1

for #16374 6b2d1a6

  • jenkins tests confirm that there are no differences as expected

@cmsbuild
Copy link
Contributor

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

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 03d9a23 into cms-sw:CMSSW_8_0_X Nov 1, 2016
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

4 participants