remove memory-churn in SiPixelTemplate2D::pushfile #40537

VinInn · 2023-01-16T15:20:10Z

PreMIx workflow was affected by a huge "memmove" storm during initialization (first event).
It was easy to locate it in SiPixelTemplate2D::pushfile.
In this purely technical PR all copies of vectors (of vectors (eventually of vectors)) have been removed.

running just initialization using

cmsDriver tt_digi --no_exec --datamix PreMix --conditions auto:phase1_2022_realistic --pileup_input file:pu.root -n 16 --era Run3 --eventcontent RAWSIM --procModifiers premix_stage2 --filein file:gensim.root -s DIGI,DATAMIX,L1,DIGI2RAW,HLT:@relval2022 --datatier GEN-SIM-DIGI-RAW --geometry DB:Extended --nThreads 8 --suffix -j JobReport.xml --customise Validation/Performance/TimeMemorySummary.customiseWithTimeMemorySummary

memory allocation is reduced
from
5017054 kmem:mm_page_alloc
5835471 kmem:mm_page_free
to
1341162 kmem:mm_page_alloc
2126517 kmem:mm_page_free

while the system time goes down from
61.298107000 seconds
to
21.264676000 seconds

I made minimal changes preserving previous behavior.
I leave to code owners any eventual further cleanup.

No change expected.

VinInn · 2023-01-16T15:21:09Z

CondFormats/SiPixelObjects/interface/SiPixel2DTemplateDBObject.h

@@ -61,7 +61,7 @@ class SiPixel2DTemplateDBObject {
  int maxIndex() const { return maxIndex_; }
  int numOfTempl() const { return numOfTempl_; }
  float version() const { return version_; }
-  std::vector<float> sVector() const { return sVector_; }
+  std::vector<float> const& sVector() const { return sVector_; }


to be honest this is the ONLY change that makes a real difference

If code-owners prefer: just keep this.

VinInn · 2023-01-16T15:23:36Z

The ultimate optimization should be to create an EsProduct with what produced by pushFile to avoid to have a copy for each producer (in each thread)

cmsbuild · 2023-01-16T15:27:19Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40537/33759

This PR adds an extra 28KB to repository

VinInn · 2023-01-16T15:29:15Z

@cmsbuild please test

VinInn · 2023-01-16T15:29:47Z

one needs to test digit premix (not sure is included in default tests)

cmsbuild · 2023-01-16T16:00:25Z

A new Pull Request was created by @VinInn (Vincenzo Innocente) for master.

It involves the following packages:

CondFormats/SiPixelObjects (db, alca)
CondFormats/SiPixelTransient (db, reconstruction)

@malbouis, @yuanchao, @ChrisMisan, @clacaputo, @saumyaphor4252, @ggovi, @tvami, @mandrenguyen, @francescobrivio can you please review it and eventually sign? Thanks.
@tocheng, @mroguljic, @dkotlins, @ferencek, @mmusich, @seemasharmafnal, @tvami this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2023-01-16T16:05:23Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40537/33766

This PR adds an extra 28KB to repository

cmsbuild · 2023-01-16T18:26:11Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-45cf13/30016/summary.html
COMMIT: 9f0fe68
CMSSW: CMSSW_13_0_X_2023-01-16-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40537/30016/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline:
11634.15 step 3
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

No significant changes to the logs found
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555538
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555510
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: no differences found

ChrisMisan · 2023-01-20T10:01:32Z

type performance-improvement

ChrisMisan · 2023-01-20T10:03:39Z

+alca

technical PR, no changes in comparison

mandrenguyen · 2023-01-22T08:48:17Z

+reconstruction

tvami · 2023-01-23T16:54:45Z

test parameters:

enable_tests = profiling

tvami · 2023-01-23T16:55:56Z

@cmsbuild , please test

cmsbuild · 2023-01-23T21:11:13Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-45cf13/30129/summary.html
COMMIT: 9f0fe68
CMSSW: CMSSW_13_0_X_2023-01-23-1100/el8_amd64_gcc11
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40537/30129/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3555495
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3555473
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 211 log files, 162 edm output root files, 49 DQM output files
TriggerResults: no differences found

tvami · 2023-01-25T14:07:01Z

+db

changes in CondFormat are acceptable for the DB schema evolution

cmsbuild · 2023-01-25T14:07:27Z

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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

rappoccio · 2023-01-25T15:15:27Z

+1

Technical PR

remove memory-churn

9f0fe68

cmsbuild added this to the CMSSW_13_0_X milestone Jan 16, 2023

cmsbuild added alca-pending code-checks-pending db-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 16, 2023

VinInn commented Jan 16, 2023

View reviewed changes

cmsbuild added code-checks-approved tests-started and removed tests-pending code-checks-pending labels Jan 16, 2023

cmsbuild added tests-approved and removed tests-started labels Jan 16, 2023

cmsbuild added performance-improvements trk labels Jan 20, 2023

cmsbuild added alca-approved and removed alca-pending labels Jan 20, 2023

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jan 22, 2023

cmsbuild added tests-started and removed tests-approved labels Jan 23, 2023

cmsbuild added tests-approved and removed tests-started labels Jan 23, 2023

cmsbuild added db-approved fully-signed and removed db-pending pending-signatures labels Jan 25, 2023

cmsbuild added orp-approved and removed orp-pending labels Jan 25, 2023

cmsbuild merged commit 9bf8071 into cms-sw:master Jan 25, 2023

VinInn mentioned this pull request Jul 27, 2023

shall we make TCmalloc default (in place of JeMalloc)? #42387

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove memory-churn in SiPixelTemplate2D::pushfile #40537

remove memory-churn in SiPixelTemplate2D::pushfile #40537

VinInn commented Jan 16, 2023

VinInn Jan 16, 2023

VinInn Jan 16, 2023 •

edited

VinInn commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

VinInn commented Jan 16, 2023

VinInn commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

ChrisMisan commented Jan 20, 2023

ChrisMisan commented Jan 20, 2023

mandrenguyen commented Jan 22, 2023

tvami commented Jan 23, 2023

tvami commented Jan 23, 2023

cmsbuild commented Jan 23, 2023

tvami commented Jan 25, 2023

cmsbuild commented Jan 25, 2023

rappoccio commented Jan 25, 2023

remove memory-churn in SiPixelTemplate2D::pushfile #40537

remove memory-churn in SiPixelTemplate2D::pushfile #40537

Conversation

VinInn commented Jan 16, 2023

VinInn Jan 16, 2023

Choose a reason for hiding this comment

VinInn Jan 16, 2023 • edited

Choose a reason for hiding this comment

VinInn commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

VinInn commented Jan 16, 2023

VinInn commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

cmsbuild commented Jan 16, 2023

Comparison Summary

ChrisMisan commented Jan 20, 2023

ChrisMisan commented Jan 20, 2023

mandrenguyen commented Jan 22, 2023

tvami commented Jan 23, 2023

tvami commented Jan 23, 2023

cmsbuild commented Jan 23, 2023

Comparison Summary

tvami commented Jan 25, 2023

cmsbuild commented Jan 25, 2023

rappoccio commented Jan 25, 2023

VinInn Jan 16, 2023 •

edited