Improve memory usage in ParameterSet #42742

Dr15Jones · 2023-09-08T19:55:57Z

PR description:

changed encoding of strings to decrease space used
use std::string_view to reduce memory churn while parsing ParameterSets
avoid use of temporary std::vector to hold elements and instead elements as needed
added additional unit tests

PR validation:

Code compiles and all framework unit tests pass.

- store strings directly with trailing \0 instead of converting to hex values first. - replaced cppunit with catch2 test

cmsbuild · 2023-09-08T20:01:53Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42742/36834

This PR adds an extra 84KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/ParameterSet/test/BuildFile.xml modified in PR(s): Improve command-line argument support for Python configs in cmsRun and edm tools #42650

cmsbuild · 2023-09-08T20:02:18Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

FWCore/Framework (core)
FWCore/Integration (core)
FWCore/ParameterSet (core)
IOPool/Common (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks.
@makortel, @missirol, @wddgit this is something you requested to watch as well.
@rappoccio, @antoniovilela, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2023-09-08T20:02:26Z

please test

cmsbuild · 2023-09-08T23:59:28Z

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cfe38f/34671/summary.html
COMMIT: a286de9
CMSSW: CMSSW_13_3_X_2023-09-08-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42742/34671/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 24 errors in the following unit tests:

---> test PrimaryVertex had ERRORS
---> test testTriggerMonitors had ERRORS
---> test testSSTGainPCL_fromRECO had ERRORS
and more ...

RelVals

135.4

----- Begin Fatal Exception 09-Sep-2023 01:09:42 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=ConcreteChargedCandidateProducer label='ElTrackCands'
Exception Message:
EntryError can not convert representation of particleType to value of type PdtEntry. Please, provide a parameter either of type int32 or string.
----- End Fatal Exception -------------------------------------------------

5.1

----- Begin Fatal Exception 09-Sep-2023 01:09:42 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=ConcreteChargedCandidateProducer label='ElTrackCands'
Exception Message:
EntryError can not convert representation of particleType to value of type PdtEntry. Please, provide a parameter either of type int32 or string.
----- End Fatal Exception -------------------------------------------------

4.22

----- Begin Fatal Exception 09-Sep-2023 01:10:00 CEST-----------------------
An exception of category 'TriggerNamesNotFound' occurred while
   [0] Processing  Event run: 160960 lumi: 277 event: 10001082 stream: 0
   [1] Running path 'dqmoffline_step'
   [2] Calling method for module CastorMonitorModule/'castorOfflineMonitor'
Exception Message:
TriggerNames not found in ParameterSet registry
----- End Fatal Exception -------------------------------------------------

Expand to see more relval errors ...

RelVals-INPUT

4.224.22_RunCosmics2011A/step2_RunCosmics2011A.log
134.813134.813_RunCosmics2015C/step2_RunCosmics2015C.log
136.733136.733_RunCosmics2016B/step2_RunCosmics2016B.log

Expand to see more relval errors ...

AddOn Tests

fastsim

----- Begin Fatal Exception 09-Sep-2023 01:09:18 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=ChargedRefCandidateProducer label='initialStepTrackRefsForJets'
Exception Message:
EntryError can not convert representation of particleType to value of type PdtEntry. Please, provide a parameter either of type int32 or string.
----- End Fatal Exception -------------------------------------------------

fastsim1

----- Begin Fatal Exception 09-Sep-2023 01:09:18 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=ChargedRefCandidateProducer label='initialStepTrackRefsForJets'
Exception Message:
EntryError can not convert representation of particleType to value of type PdtEntry. Please, provide a parameter either of type int32 or string.
----- End Fatal Exception -------------------------------------------------

fastsim2

----- Begin Fatal Exception 09-Sep-2023 01:09:18 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=ChargedRefCandidateProducer label='initialStepTrackRefsForJets'
Exception Message:
EntryError can not convert representation of particleType to value of type PdtEntry. Please, provide a parameter either of type int32 or string.
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

cmsbuild · 2023-09-09T14:24:48Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42742/36835

This PR adds an extra 20KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/ParameterSet/test/BuildFile.xml modified in PR(s): Improve command-line argument support for Python configs in cmsRun and edm tools #42650

cmsbuild · 2023-09-27T03:50:38Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cfe38f/34920/summary.html
COMMIT: 68c41f7
CMSSW: CMSSW_13_3_X_2023-09-26-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42742/34920/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 2 errors in the following unit tests:

---> test TestRunnerFWCoreTFWLiteSelector had ERRORS
---> test CondToolsLHCInfoNewPopConTest had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3356848
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3356823
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -308.312 KiB( 49 files compared)
DQMHistoSizes: changed ( 1000.0 ): -308.312 KiB HLT/EGM
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

Dr15Jones · 2023-09-27T14:35:27Z

Please test

cmsbuild · 2023-09-27T19:58:18Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cfe38f/34940/summary.html
COMMIT: 68c41f7
CMSSW: CMSSW_13_3_X_2023-09-27-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42742/34940/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 1 errors in the following unit tests:

---> test CondToolsLHCInfoNewPopConTest had ERRORS

Comparison Summary

Summary:

You potentially removed 5 lines from the logs
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3357124
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3357096
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -308.312 KiB( 49 files compared)
DQMHistoSizes: changed ( 1000.0 ): -308.312 KiB HLT/EGM
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

makortel · 2023-09-27T20:29:47Z

The CondToolsLHCInfoNewPopConTest fails also in the IBs.

makortel · 2023-10-03T15:14:44Z

+core

cmsbuild · 2023-10-03T15:15:10Z

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

antoniovilela · 2023-10-04T00:57:29Z

+1

antoniovilela · 2023-10-04T00:57:40Z

merge

Caused by backporting cms-sw#42742 on top of cms-sw#43898

Dr15Jones added 5 commits September 7, 2023 15:11

Simplify encoding of string in edm::ParameterSet

490396f

- store strings directly with trailing \0 instead of converting to hex values first. - replaced cppunit with catch2 test

Add test for decoding old string encoding

3f3a8e2

Update tests that depend on PSet ID values

ec59f7d

Use std::string_view where appropriate

3671562

Added an edm::split which tokenizes one at a time

a286de9

cmsbuild added this to the CMSSW_13_3_X milestone Sep 8, 2023

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Sep 8, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 8, 2023

cmsbuild added tests-started and removed tests-pending labels Sep 8, 2023

makortel mentioned this pull request Sep 8, 2023

Improve string encoding done in ParameterSet cms-sw/framework-team#662

Closed

cmsbuild added tests-rejected and removed tests-started labels Sep 8, 2023

Dr15Jones added 2 commits September 9, 2023 09:15

Added tests for existsAs for string and vstring

3da8a15

Updated getParameter for PdtEntry for new string handling

4b961fa

cmsbuild added simulation-pending tests-pending code-checks-pending and removed tests-rejected code-checks-approved labels Sep 9, 2023

cmsbuild added tests-rejected and removed tests-started labels Sep 27, 2023

cmsbuild added tests-started and removed tests-rejected labels Sep 27, 2023

cmsbuild added tests-rejected and removed tests-started labels Sep 27, 2023

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Oct 3, 2023

cmsbuild added orp-approved and removed orp-pending labels Oct 4, 2023

cmsbuild merged commit b714992 into cms-sw:master Oct 4, 2023
1 check passed

cmsbuild mentioned this pull request Oct 4, 2023

Updated root to tip of branch master cms-sw/cmsdist#8734

Closed

Dr15Jones deleted the newStringParameterSet branch October 6, 2023 14:22

dan131riley mentioned this pull request Jan 29, 2024

unhelpful assertion failure reading new files with older releases #43807

Closed

makortel mentioned this pull request Jan 29, 2024

Issue clear error message for incompatible ParameterSet encoding cms-sw/framework-team#804

Closed

7 tasks

makortel mentioned this pull request May 6, 2024

Allow 14_0_X files to be read in 13_0_X and 12_4_X cms-sw/framework-team#917

Closed

4 tasks

makortel added a commit to makortel/cmssw that referenced this pull request May 7, 2024

Fix compilation

5916b34

Caused by backporting cms-sw#42742 on top of cms-sw#43898

makortel mentioned this pull request May 7, 2024

[13_0_HLT_X] Improve memory usage in ParameterSet #44921

Merged

makortel added a commit to makortel/cmssw that referenced this pull request May 7, 2024

Fix compilation

74905fa

Caused by backporting cms-sw#42742 on top of cms-sw#43898

makortel mentioned this pull request May 17, 2024

[12_4_HLT_X] Improve memory usage in ParameterSet #45000

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve memory usage in ParameterSet #42742

Improve memory usage in ParameterSet #42742

Dr15Jones commented Sep 8, 2023

cmsbuild commented Sep 8, 2023

cmsbuild commented Sep 8, 2023

Dr15Jones commented Sep 8, 2023

cmsbuild commented Sep 8, 2023

cmsbuild commented Sep 9, 2023

cmsbuild commented Sep 27, 2023

Dr15Jones commented Sep 27, 2023

cmsbuild commented Sep 27, 2023

makortel commented Sep 27, 2023

makortel commented Oct 3, 2023

cmsbuild commented Oct 3, 2023

antoniovilela commented Oct 4, 2023

antoniovilela commented Oct 4, 2023

Improve memory usage in ParameterSet #42742

Improve memory usage in ParameterSet #42742

Conversation

Dr15Jones commented Sep 8, 2023

PR description:

PR validation:

cmsbuild commented Sep 8, 2023

cmsbuild commented Sep 8, 2023

Dr15Jones commented Sep 8, 2023

cmsbuild commented Sep 8, 2023

Unit Tests

RelVals

RelVals-INPUT

AddOn Tests

cmsbuild commented Sep 9, 2023

cmsbuild commented Sep 27, 2023

Unit Tests

Comparison Summary

Dr15Jones commented Sep 27, 2023

cmsbuild commented Sep 27, 2023

Unit Tests

Comparison Summary

makortel commented Sep 27, 2023

makortel commented Oct 3, 2023

cmsbuild commented Oct 3, 2023

antoniovilela commented Oct 4, 2023

antoniovilela commented Oct 4, 2023