Use ServiceWeakToken with asynchronous tasks #33330

Dr15Jones · 2021-04-02T19:23:24Z

PR description:

PR validation:

All framework unit tests pass.

Make sure ServiceToken remains alive during asynchronous calls.

To avoid the case where a tbb task lives beyond the lifetime of the EventProcessor, we need to avoid having the Services being held alive by the lambdas.

Dr15Jones · 2021-04-02T19:25:06Z

@dan131riley this is the implementation I finally decided upon, replacing the one to which I pointed you.

cmsbuild · 2021-04-02T19:30:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33330/21880

This PR adds an extra 96KB to repository

cmsbuild · 2021-04-02T19:30:35Z

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

It involves the following packages:

FWCore/Framework
FWCore/ServiceRegistry

@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks.
@makortel, @wddgit, @fwyzard this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2021-04-02T19:32:49Z

please test

cmsbuild · 2021-04-03T01:45:31Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cd92ed/13983/summary.html
COMMIT: 2fabe96
CMSSW: CMSSW_11_3_X_2021-04-02-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33330/13983/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:

Move to D76 in HGCal geometry unit test #33326 @srimanob: Move to D76 in HGCal geometry unit test
move kalmanLUTs.root to cms-data external #33304 @smuzaffar: move kalmanLUTs.root to cms-data external

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

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2640868
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2640839
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 36 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

…s-merge-topic

Disable TBB thread observer in PostEndJob watcher

cmsbuild · 2021-04-04T15:38:52Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33330/21882

This PR adds an extra 120KB to repository

cmsbuild · 2021-04-04T15:39:12Z

Pull request #33330 was updated. @smuzaffar, @Dr15Jones, @makortel, @cmsbuild, @Martin-Grunewald, @fwyzard can you please check and sign again.

Dr15Jones · 2021-04-04T15:48:45Z

please test

cmsbuild · 2021-04-04T20:27:31Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cd92ed/13991/summary.html
COMMIT: 9cf7351
CMSSW: CMSSW_11_3_X_2021-04-04-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33330/13991/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2640868
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2640845
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

Martin-Grunewald · 2021-04-05T08:20:33Z

+1

makortel · 2021-04-05T14:05:49Z

FWCore/Services/plugins/InitRootHandlers.cc

@@ -771,6 +771,11 @@ namespace edm {

      if (not threadTracker_) {
        threadTracker_ = std::make_unique<ThreadTracker>();
+        iReg.watchPostEndJob([]() {
+          if (threadTracker_) {


I'm wondering how this gets captured with empty [] (even if it appears to compile here).

weird indeed

threadTracker_ is static so no this pointer is needed.

makortel · 2021-04-05T14:11:02Z

+1

cmsbuild · 2021-04-05T14:11:28Z

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

qliphy · 2021-04-05T14:35:17Z

+1

Dr15Jones added 2 commits April 2, 2021 10:42

Extend lifetime of ServiceToken in unit tests

20aca84

Make sure ServiceToken remains alive during asynchronous calls.

Use ServiceWeakToken with lambdas

2fabe96

To avoid the case where a tbb task lives beyond the lifetime of the EventProcessor, we need to avoid having the Services being held alive by the lambdas.

cmsbuild added this to the CMSSW_11_3_X milestone Apr 2, 2021

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Apr 2, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 2, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 2, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 3, 2021

Merged tryServiceWeakTokenFixForTBB from repository Dr15Jones with cm…

311fe69

…s-merge-topic

dan131riley mentioned this pull request Apr 4, 2021

Random multithreaded test failures in FWCore #33289

Closed

Dan Riley and others added 2 commits April 4, 2021 10:41

Disable thread observer after EndJob

d0c3d42

Merge pull request #6 from dan131riley/tbb-observer-shutdown2

9cf7351

Disable TBB thread observer in PostEndJob watcher

cmsbuild added code-checks-pending hlt-pending tests-pending and removed code-checks-approved tests-approved labels Apr 4, 2021

cmsbuild removed the code-checks-pending label Apr 4, 2021

cmsbuild added the code-checks-approved label Apr 4, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 4, 2021

cmsbuild mentioned this pull request Apr 4, 2021

RepeatingCachedRootSource #33332

Merged

cmsbuild added tests-approved and removed tests-started labels Apr 4, 2021

cmsbuild added hlt-approved and removed hlt-pending labels Apr 5, 2021

cmsbuild mentioned this pull request Apr 5, 2021

Addition of the MTD to the tracker Navigation #33272

Merged

makortel reviewed Apr 5, 2021

View reviewed changes

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Apr 5, 2021

makortel mentioned this pull request Apr 5, 2021

Use ServiceWeakToken with asynchronous tasks cms-sw/framework-team#106

Closed

cmsbuild added orp-approved and removed orp-pending labels Apr 5, 2021

cmsbuild merged commit fbb75ea into cms-sw:master Apr 5, 2021

Dr15Jones deleted the tryServiceWeakTokenFixForTBB branch April 19, 2021 16:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ServiceWeakToken with asynchronous tasks #33330

Use ServiceWeakToken with asynchronous tasks #33330

Dr15Jones commented Apr 2, 2021

Dr15Jones commented Apr 2, 2021

cmsbuild commented Apr 2, 2021

cmsbuild commented Apr 2, 2021

Dr15Jones commented Apr 2, 2021

cmsbuild commented Apr 3, 2021

cmsbuild commented Apr 4, 2021

cmsbuild commented Apr 4, 2021

Dr15Jones commented Apr 4, 2021

cmsbuild commented Apr 4, 2021

Martin-Grunewald commented Apr 5, 2021

makortel Apr 5, 2021 •

edited

Dr15Jones Apr 5, 2021

Dr15Jones Apr 5, 2021 •

edited

makortel Apr 5, 2021

makortel commented Apr 5, 2021

cmsbuild commented Apr 5, 2021

qliphy commented Apr 5, 2021

Use ServiceWeakToken with asynchronous tasks #33330

Use ServiceWeakToken with asynchronous tasks #33330

Conversation

Dr15Jones commented Apr 2, 2021

PR description:

PR validation:

Dr15Jones commented Apr 2, 2021

cmsbuild commented Apr 2, 2021

cmsbuild commented Apr 2, 2021

Dr15Jones commented Apr 2, 2021

cmsbuild commented Apr 3, 2021

Comparison Summary

cmsbuild commented Apr 4, 2021

cmsbuild commented Apr 4, 2021

Dr15Jones commented Apr 4, 2021

cmsbuild commented Apr 4, 2021

Comparison Summary

Martin-Grunewald commented Apr 5, 2021

makortel Apr 5, 2021 • edited

Choose a reason for hiding this comment

Dr15Jones Apr 5, 2021

Choose a reason for hiding this comment

Dr15Jones Apr 5, 2021 • edited

Choose a reason for hiding this comment

makortel Apr 5, 2021

Choose a reason for hiding this comment

makortel commented Apr 5, 2021

cmsbuild commented Apr 5, 2021

qliphy commented Apr 5, 2021

makortel Apr 5, 2021 •

edited

Dr15Jones Apr 5, 2021 •

edited