[CPP20] Remove volatile from CondCore/Utilities #45104

francescobrivio · 2024-05-30T14:07:16Z

PR description:

Addressing #45072
This PR removes the only occurrence of a volatile variable in CondCore/Utilities.
The cpp script containing such variable is not run in production and not even run as unitTest.

The [RFC] in the title is due to the fact that I'm not 100% sure this is the correct way of handling this, at least I could not understand if there are preferred ways to address this deprecation from the link in the original issue, but I don't have much experience with this. 😄
@iarspider @VinInn any feedback is more than welcome.

PR validation:

Code compiles.
Additionally, I have run the following command:

conddb_test_gt_perf -g 80X_dataRun2_HLT_for800Validatio_test -n 1 -c oracle://cms_orcoff_prep/CMS_CONDITIONS --n_fetch 1 --n_deser 1 2>&1 | tee run.log

with and without this PR and the test passes with the same result.

Backport:

Not a backport, no backport needed

cmsbuild · 2024-05-30T14:07:39Z

cms-bot internal usage

francescobrivio · 2024-05-30T14:08:00Z

@cmsbuild please test for CMSSW_14_1_CPP20_X

cmsbuild · 2024-05-30T14:15:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45104/40432

This PR adds an extra 20KB to repository

cmsbuild · 2024-05-30T14:15:34Z

A new Pull Request was created by @francescobrivio for master.

It involves the following packages:

CondCore/Utilities (db)

@saumyaphor4252, @perrotta, @consuegs, @francescobrivio can you please review it and eventually sign? Thanks.
@PonIlya, @yuanchao, @rsreds, @JanChyczynski, @mmusich 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

VinInn · 2024-05-30T15:33:47Z

CondCore/Utilities/bin/conddb_test_gt_perf.cpp

@@ -325,7 +325,7 @@ cond::TestGTPerf::TestGTPerf() : Utilities("conddb_test_gt_load") {
 // thread helpers

 // global counter for dummy thread measurements:
-volatile int fooGlobal = 0;
+int fooGlobal = 0;


I suspect this should be std::atomic<int>

Thanks Vincenzo! I'll go with that then.

cmsbuild · 2024-05-30T17:58:13Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8245ec/39630/summary.html
COMMIT: 8727209
CMSSW: CMSSW_14_1_CPP20_X_2024-05-27-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45104/39630/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

@mbluj [DBG] Fix undefined ref errors in L1TMuonOverlapPhase2 #44984
@abdoulline HcalZDCDetId fix for Run3 #45033
@mbluj Set 3 instead of 4 OMTF track sub-address as expected by uGMT #45030
@bsunanda Phase2-hgx356 Fix the bugs in HGCal TB simulation for 2023 scenarios #45036
@cms-sw clang-tidy: Remove deprecated AnalyzeTemporaryDtors option #45034
@stahlleiton Add new dEdx calibration condition format #45024
@iarspider Fix undeclared identifiers in RectangularPixelPhase2Topology #45038
@bsunanda Run3-alca247A Bug fix to the code Calibration/HcalCalibAlgos/plugins/HcalIsoTrkAnalyzer.cc for initialiting uncorrection of RecHit energies #45046
@bsunanda Run3-alca247Z Add a new defintion of truncated DetID and a new scaling script for HCAL IsoTrack calibration #45044
@bsunanda Run3-alca247YX Add examples for the IsoTrack calibration in Calibration/HcalCalibAlgos/macros/jobs (restore the material from #45010) #45045

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8245ec/39630/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8245ec/39630/git-merge-result

Comparison Summary

Summary:

You potentially added 2561 lines to the logs
Reco comparison results: 122 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3338862
DQMHistoTests: Total failures: 956
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3337886
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

cmsbuild · 2024-05-31T08:23:47Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45104/40441

This PR adds an extra 20KB to repository

cmsbuild · 2024-05-31T08:24:07Z

Pull request #45104 was updated. @cmsbuild, @francescobrivio, @consuegs, @saumyaphor4252, @perrotta can you please check and sign again.

francescobrivio · 2024-05-31T08:27:05Z

please test for CMSSW_14_1_CPP20_X

cmsbuild · 2024-05-31T12:21:28Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8245ec/39639/summary.html
COMMIT: 8c32195
CMSSW: CMSSW_14_1_CPP20_X_2024-05-27-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45104/39639/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

@mbluj [DBG] Fix undefined ref errors in L1TMuonOverlapPhase2 #44984
@abdoulline HcalZDCDetId fix for Run3 #45033
@mbluj Set 3 instead of 4 OMTF track sub-address as expected by uGMT #45030
@bsunanda Phase2-hgx356 Fix the bugs in HGCal TB simulation for 2023 scenarios #45036
@cms-sw clang-tidy: Remove deprecated AnalyzeTemporaryDtors option #45034
@stahlleiton Add new dEdx calibration condition format #45024
@iarspider Fix undeclared identifiers in RectangularPixelPhase2Topology #45038
@bsunanda Run3-alca247A Bug fix to the code Calibration/HcalCalibAlgos/plugins/HcalIsoTrkAnalyzer.cc for initialiting uncorrection of RecHit energies #45046
@bsunanda Run3-alca247Z Add a new defintion of truncated DetID and a new scaling script for HCAL IsoTrack calibration #45044
@bsunanda Run3-alca247YX Add examples for the IsoTrack calibration in Calibration/HcalCalibAlgos/macros/jobs (restore the material from #45010) #45045

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8245ec/39639/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8245ec/39639/git-merge-result

Comparison Summary

Summary:

You potentially added 2566 lines to the logs
Reco comparison results: 124 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3338862
DQMHistoTests: Total failures: 962
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3337880
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

perrotta · 2024-06-01T13:55:33Z

please test

cmsbuild · 2024-06-01T16:37:03Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8245ec/39660/summary.html
COMMIT: 8c32195
CMSSW: CMSSW_14_1_X_2024-06-01-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45104/39660/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3338862
DQMHistoTests: Total failures: 9
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3338833
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

perrotta · 2024-06-07T11:55:12Z

+db

cmsbuild · 2024-06-07T11:55:36Z

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

rappoccio · 2024-06-12T01:30:47Z

+1

perrotta · 2024-06-12T05:35:52Z

@cmsbuild ping

iarspider · 2024-06-12T11:37:26Z

@cmsbuild ping

cmsbuild added this to the CMSSW_14_1_X milestone May 30, 2024

cmsbuild added db-pending pending-signatures tests-pending orp-pending code-checks-pending labels May 30, 2024

cmsbuild added tests-started and removed tests-pending labels May 30, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels May 30, 2024

VinInn reviewed May 30, 2024

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels May 30, 2024

Remove volatile from CondCore/Utilities

8c32195

francescobrivio force-pushed the alca_remove_volatile branch from 8727209 to 8c32195 Compare May 31, 2024 08:18

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels May 31, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels May 31, 2024

cmsbuild removed the tests-pending label May 31, 2024

cmsbuild added the tests-started label May 31, 2024

cmsbuild added tests-approved and removed tests-started labels May 31, 2024

cmsbuild added tests-started and removed tests-approved labels Jun 1, 2024

cmsbuild added tests-approved and removed tests-started labels Jun 1, 2024

francescobrivio changed the title ~~[RFC] [CPP20] Remove volatile from CondCore/Utilities~~ [CPP20] Remove volatile from CondCore/Utilities Jun 5, 2024

cmsbuild added fully-signed db-approved and removed db-pending pending-signatures labels Jun 7, 2024

cmsbuild added orp-approved and removed orp-pending labels Jun 12, 2024

cmsbuild merged commit 3ed2913 into cms-sw:master Jun 12, 2024
17 checks passed

This was referenced Jun 12, 2024

[AARCH64] jemalloc: explicitly set pagesize cms-sw/cmsdist#9243

Merged

StreamSchedule.h: remove unused variable #45205

Merged

francescobrivio deleted the alca_remove_volatile branch June 26, 2024 08:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CPP20] Remove volatile from CondCore/Utilities #45104

[CPP20] Remove volatile from CondCore/Utilities #45104

francescobrivio commented May 30, 2024

cmsbuild commented May 30, 2024 •

edited

Loading

francescobrivio commented May 30, 2024

cmsbuild commented May 30, 2024

cmsbuild commented May 30, 2024

VinInn May 30, 2024

francescobrivio May 31, 2024

cmsbuild commented May 30, 2024

cmsbuild commented May 31, 2024

cmsbuild commented May 31, 2024

francescobrivio commented May 31, 2024

cmsbuild commented May 31, 2024

perrotta commented Jun 1, 2024

cmsbuild commented Jun 1, 2024

perrotta commented Jun 7, 2024

cmsbuild commented Jun 7, 2024

rappoccio commented Jun 12, 2024

perrotta commented Jun 12, 2024

iarspider commented Jun 12, 2024

[CPP20] Remove volatile from CondCore/Utilities #45104

[CPP20] Remove volatile from CondCore/Utilities #45104

Conversation

francescobrivio commented May 30, 2024

PR description:

PR validation:

Backport:

cmsbuild commented May 30, 2024 • edited Loading

francescobrivio commented May 30, 2024

cmsbuild commented May 30, 2024

cmsbuild commented May 30, 2024

VinInn May 30, 2024

Choose a reason for hiding this comment

francescobrivio May 31, 2024

Choose a reason for hiding this comment

cmsbuild commented May 30, 2024

Comparison Summary

cmsbuild commented May 31, 2024

cmsbuild commented May 31, 2024

francescobrivio commented May 31, 2024

cmsbuild commented May 31, 2024

Comparison Summary

perrotta commented Jun 1, 2024

cmsbuild commented Jun 1, 2024

Comparison Summary

perrotta commented Jun 7, 2024

cmsbuild commented Jun 7, 2024

rappoccio commented Jun 12, 2024

perrotta commented Jun 12, 2024

iarspider commented Jun 12, 2024

cmsbuild commented May 30, 2024 •

edited

Loading