Added ROOT dictionaries in DataFormats - #51739
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51739/50669
|
|
A new Pull Request was created by @alexstrel for master. It involves the following packages:
@BenjaminRS, @Moanwar, @cmsbuild, @jfernan2, @mandrenguyen, @quinnanm, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
makortel
left a comment
There was a problem hiding this comment.
Few suggestions to have related types closer each other in the XML file. But please wait for the build of #51200 (comment) to succeed before updating this PR.
| @@ -1,5 +1,6 @@ | |||
| <lcgdict> | |||
|
|
|||
| <class name="std::vector<reco::io_v1::ElectronSeed::PMVars>"/> | |||
There was a problem hiding this comment.
Please move this line below
| </class> | ||
| <class name="std::vector<l1t::io_v1::TkEm>"/> | ||
| <class name="edm::Wrapper<std::vector<l1t::io_v1::TkEm> >"/> | ||
| <class name="edm::refhelper::FindUsingAdvance<std::vector<l1t::io_v1::TkElectron>,l1t::io_v1::TkElectron>"/> |
There was a problem hiding this comment.
Please move this above the line
| <class name="l1t::RegionalOutput<l1t::PFCandidateCollection>" /> | ||
| <class name="edm::Wrapper<l1t::RegionalOutput<l1t::PFCandidateCollection>>" /> | ||
|
|
||
| <class name="edm::refhelper::FindUsingAdvance<std::vector<l1t::io_v1::PFJet>,l1t::io_v1::PFJet>"/> |
There was a problem hiding this comment.
Please move this above the line
| <class name="std::pair<reco::isodeposit::Direction::Distance,float>"/> | ||
| <class name="std::map<reco::isodeposit::Direction::Distance,float>"/> |
There was a problem hiding this comment.
Please move these below the line
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51739/50681
|
|
Pull request #51739 was updated. @BenjaminRS, @Moanwar, @cmsbuild, @jfernan2, @mandrenguyen, @quinnanm, @srimanob can you please check and sign again. |
|
@cmsbuild, please test |
|
+1 Size: This PR adds an extra 16KB to repository DAS Queries: The DAS query tests failed, see the summary page for details. Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 21 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
|
@cms-sw/l1-l2 @cms-sw/reconstruction-l2 Note that strictly speaking this PR is independent from #51200 and can (IMHO should) be reviewed and merged separately. |
|
Hi @alexstrel, thanks a lot for the PR! It’s nice to see the reduction in memory usage. Just for my understanding, is this reduction expected? And why is it specifically in Step 5? It’s a little bit unclear to me. |
Generally more compiled dictionaries for data types that ROOT serializes as opposed to header parsing can reduce memory. In several of the workflows the step 5 is ALCA. Could be related to the data types that are being read/written there. |
Then I would be greedy and ask if we can add that to more DataFormats packages ? |
Add what exactly? |
Well I see this changes cover several DataFormats packages, including BTauReco, EgammaCandidates, EgammaReco, HcalIsolatedTrack, L1TCorrelator, L1TMuon, L1TParticleFlow, ParticleFlowReco, and RecoCandidate. So can it cover more ? This my question |
Theoretically that would be great, but practice is more complicated. These cases were found by running a job on a CMSSW build that was built on top of a ROOT debug build in I'd love to have a better way to find missing dictionaries (see #49458), but we are not there yet. |
|
+1 |
|
+l1 |
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @mandrenguyen, @ftenchini (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
This PR adds ROOT dictionary instantiations identified during the investigation of ROOT autoparsing and memory usage. The changes cover several DataFormats packages, including
BTauReco,EgammaCandidates,EgammaReco,HcalIsolatedTrack,L1TCorrelator,L1TMuon,L1TParticleFlow,ParticleFlowReco, andRecoCandidate.The goal is to provide the required dictionaries explicitly and avoid unnecessary runtime autoparsing.
Resolves cms-sw/framework-team#2398