Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

During event processing only call const methods of Principal #12936

Merged
merged 6 commits into from Jan 16, 2016
Merged

During event processing only call const methods of Principal #12936

merged 6 commits into from Jan 16, 2016

Conversation

Dr15Jones
Copy link
Contributor

During the portion of the event loop where the framework is interacting with modules we now only call const methods of the Principal. Therefore to make it safe to use multiple threads per event we just need to make Principal const thread-safe.

@Dr15Jones
Only use Principal const interfaces when processing modules
17e4d05 
In order to support multiple threads interacting with one Event we need a way to identify those methods which must be thread-safe. Since the multiple threads would only happen when doing module processing, all such interactions now occur via the const interface of the Principals and their inheriting classes. This means we only have to make Principals const thread-safe.
@Dr15Jones
Moved readImmediate functionality to Principal
d7add21 
LuminosityBlockPrincipal and RunPrincipal had identical implementations for the function readImmediate. This was moved to Principal base class and renamed readAllFromSourceAndMergeImmediately
@Dr15Jones
Added unsafe_ methods to help with threading changes to ProductData
466bf80 
Added specific unsafe_ methods which are const methods that change the state of the object. These are ones that will have to be modified to become thread-safe.
@Dr15Jones
Only call const functions of ProductHolders during Path processing
f24a76f 
When we are processing Paths during the event loop we only want to call thread-safe functions of ProductHolders. By switching to only calling 'const' functions during that time we can concentrate on making ProductHolders const thread-safe.
@Dr15Jones
Use propagate_const on ProductHolderBase container in Principal
fbb13be 
Switched the Principal to use a container using the propatage_const wrapper so we can guarantee we call only const methods of a ProductHolderBase.
@Dr15Jones
Added back T_EventSetup_PHGCalParameters.cc
cfe0481 
Somehow this file was deleted as part of an earlier commit
@cmsbuild cmsbuild added this to the Next CMSSW_8_0_X milestone Jan 13, 2016
@cmsbuild
Copy link
Contributor

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

It involves the following packages:

DataFormats/Common
FWCore/Framework
FWCore/Modules
IOPool/Input

@cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @wddgit, @wmtan this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@Dr15Jones
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10488/console

@cmsbuild
Copy link
Contributor

-1

Tested at: cfe0481
I found errors in the following unit tests:

---> test runtestRecoEgammaPhotonIdentification had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12936/10488/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
6b616ea
5c6850f
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12936/10488/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12936/10488/git-merge-result

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12936/10488/summary.html

@Dr15Jones
Copy link
Contributor Author

The test failure is not from this pull request. The test failed because of an Xrootd time out failure.

@Dr15Jones
Copy link
Contributor Author

Please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10500/console

@Dr15Jones
Copy link
Contributor Author

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

-1

Tested at: cfe0481
I found errors in the following unit tests:

---> test runtestRecoEgammaPhotonIdentification had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12936/10500/summary.html

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12936/10500/summary.html

@Dr15Jones
Copy link
Contributor Author

The same xrootd fallback errors happened

----- Begin Fatal Exception 14-Jan-2016 17:54:10 CET-----------------------
An exception of category 'FallbackFileOpenError' occurred while
   [0] Calling InputSource::readFile_
   [1] Calling RootFileSequenceBase::initTheFile()
   [2] Calling StorageFactory::open()
   [3] Calling XrdFile::open()
Exception Message:
Failed to open the file 'root://xrootd-cms.infn.it//store/relval/CMSSW_7_6_0_pre4/RelValH130GGgluonfusion_13/GEN-SIM-RECO/PU25ns_76X_mcRun2_asymptotic_v1-v1/00000/18C42D26-714F-E511-90A9-0025905B855C.root'
   Additional Info:

@Dr15Jones
Copy link
Contributor Author

@davidlange6 please merge this change since the failed test is not caused by this change. I need this committed before I make additional commits. Thanks.

@wmtan
Copy link
Contributor

wmtan commented Jan 15, 2016

@davidlange6 FWIW I also need this committed before i make additional commits. Thanks.

davidlange6 added a commit that referenced this pull request Jan 16, 2016
@davidlange6
Merge pull request #12936 from Dr15Jones/constAccessPrincipals
52d8e30 
During event processing only call const methods of Principal
@davidlange6 davidlange6 merged commit 52d8e30 into cms-sw:CMSSW_8_0_X Jan 16, 2016
cmsbuild added a commit that referenced this pull request Jan 16, 2016
@cmsbuild
Merge pull request #12936 from Dr15Jones/constAccessPrincipals
cfea955
@Dr15Jones Dr15Jones deleted the constAccessPrincipals branch February 4, 2016 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comparison-available core-approved fully-signed orp-pending tests-rejected
Projects
None yet
Milestone
CMSSW_8_0_X
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Dr15Jones @cmsbuild @wmtan @davidlange6