Skip to content

Improve how some ActivityRegistry signals are emitted - #51625

Merged
cmsbuild merged 5 commits into
cms-sw:masterfrom
makortel:improveSignals
Aug 13, 2026
Merged

Improve how some ActivityRegistry signals are emitted#51625
cmsbuild merged 5 commits into
cms-sw:masterfrom
makortel:improveSignals

Conversation

@makortel

@makortel makortel commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PR description:

This PR improves how ActivityRegistry signals for the "easy cases" listed in #51545 are emitted. It adds a edm::signalslot::SignalSentry helper class to call a given lambda either at succeeded() method (exception from the lambda propagated) or at the destructor (exception from the lambda not propagated).

The overall pattern followed here is

{
  
  auto sentry = edm::signalslot::make_sentry(<lambda for post signal>);
  <emit pre signal>

  <operation itself>

  sentry.succeeded();
}

Creating the sentry for the Post signal first makes the Post signal to be called also if Pre signal throws an exception, and the succeeded() method call allows exceptions from the Post signal (even if we recommend against those) to propagate if there isn't an exception in flight already.

Resolves cms-sw/framework-team#2367

PR validation:

Unit tests pass

@cmsbuild

cmsbuild commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

cms-bot internal usage

@cmsbuild

cmsbuild commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@cmsbuild

cmsbuild commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

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

It involves the following packages:

  • CondCore/CondHDF5ESSource (db)
  • FWCore/Framework (core)
  • FWCore/Utilities (core)
  • FWIO/RNTupleTempInput (core)
  • IOPool/Input (core)

@Alejandro1400, @Dr15Jones, @EltonSh, @JanChyczynski, @arunhep, @cmsbuild, @francescobrivio, @makortel, @perrotta, @smuzaffar can you please review it and eventually sign? Thanks.
@JanChyczynski, @PonIlya, @felicepantaleo, @mmusich, @wddgit this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@makortel

makortel commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@cmsbuild, please test

@cmsbuild

cmsbuild commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Size: This PR adds an extra 124KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5fb4e8/55083/summary.html
COMMIT: 9ca3e83
CMSSW: CMSSW_20_1_X_2026-08-04-1100/el9_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/51625/55083/install.sh to create a dev area with all the needed externals and cmssw changes.

Failed Unit Tests

I found 8 errors in the following unit tests:

---> test test-das-selected-lumis had ERRORS
---> test DetectorDescriptionDDCMSTestDriver had ERRORS
---> test GeometryMTDGeometryBuilderTestDriver had ERRORS
and more ...

Failed RelVals

The relvals timed out after 4 hours.

Failed RelVals-INPUT

The relvals timed out after 4 hours.

Failed AddOn Tests

A fatal system signal has occurred: segmentation violation
----- Begin Fatal Exception 04-Aug-2026 23:14:31 CEST-----------------------
An exception of category 'BadAlloc' occurred while
   [0] Calling endJob and endStream
Exception Message:
A std::bad_alloc exception was thrown.
The job has probably exhausted the virtual memory available to the process.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 04-Aug-2026 23:14:29 CEST-----------------------
An exception of category 'BadAlloc' occurred while
   [0] Calling endJob and endStream
Exception Message:
A std::bad_alloc exception was thrown.
The job has probably exhausted the virtual memory available to the process.
----- End Fatal Exception -------------------------------------------------
Expand to see more addon errors ...

@makortel

makortel commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Very interesting failures

@makortel

makortel commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Found one mistake, preESModuleSignal_ was emitted twice

@makortel

makortel commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@cmsbuild, please test

@cmsbuild

cmsbuild commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@cmsbuild

cmsbuild commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Pull request #51625 was updated. @Alejandro1400, @Dr15Jones, @EltonSh, @JanChyczynski, @arunhep, @cmsbuild, @francescobrivio, @makortel, @perrotta, @smuzaffar can you please check and sign again.

@makortel

makortel commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@cmsbuild, please test

@cmsbuild

cmsbuild commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

-1

Failed Tests: UnitTests
Size: This PR adds an extra 32KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5fb4e8/55180/summary.html
COMMIT: 71fdefd
CMSSW: CMSSW_20_1_X_2026-08-07-1100/el9_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/51625/55180/install.sh to create a dev area with all the needed externals and cmssw changes.

Failed Unit Tests

I found 7 errors in the following unit tests:

---> test test-das-selected-lumis had ERRORS
---> test testJetMETCorrectionsType1MET had ERRORS
---> test test_edmPickEvents had ERRORS
and more ...

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 45
  • DQMHistoTests: Total histograms compared: 3589140
  • DQMHistoTests: Total failures: 16
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3589106
  • DQMHistoTests: Total skipped: 18
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 44 files compared)
  • Checked 195 log files, 163 edm output root files, 45 DQM output files
  • TriggerResults: no differences found

@makortel

makortel commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Comparison differences are related to #47071

@makortel

makortel commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Test failures are known problems or infrastructure related

@makortel

makortel commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

ignore tests-rejected with ib-failure

@makortel

Copy link
Copy Markdown
Contributor Author

+core

@perrotta

Copy link
Copy Markdown
Contributor

+db

@cmsbuild

Copy link
Copy Markdown
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (test failures were overridden). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @ftenchini, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen

Copy link
Copy Markdown
Contributor

+1

@cmsbuild
cmsbuild merged commit 8da030d into cms-sw:master Aug 13, 2026
9 of 10 checks passed
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.

Make easy improvements to ActivityRegistry signal calls

5 participants