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.

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

Thread safe fixes for DataFormats/PatCandidates #1375

Conversation

Dr15Jones
Copy link
Contributor

Changed caches used by the data products to be thread-safe. Also dealt with non-const statics.

Volker Adler and others added 9 commits November 3, 2013 22:36
Moved the calculation of the name demangling to a static function of the base class. This allowed the removal of a non-const static and avoided having to create that code for each template type. The const static name only needs call that function once per job. This will avoid a potential thread-safety issue.
…AllFilter

The class AcceptAllFilter has a method singleton method 'get' which was creating a non-const function static and returning it a const. This was replaced with a const class static so that the compiler didn't have to create code in each calling file for the function static. This avoids having the static analyzer produce a thread-safety complaint.
Used std::atomic<*> to manage a thread safe cache of information. This dropped a transient data member so classes_def_objects.xml file had to be changed.
After using AtomicPtrCache in real code, we found that a dedicated reset() method makes the code cleaner. Previously one had to create a temporary empty AtomicPtrCache and assign it to the old variable in order to reset.
Changed caches to be of type edm::AtomicPtrCache<> to avoid thread safety issues.
@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2013

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

Thread safe fixes for DataFormats/PatCandidates

It involves the following packages:

DataFormats/PatCandidates
DataFormats/Common

@cmsbuild, @vadler, @Dr15Jones, @ktf, @nclopezo can you please review it and eventually sign? Thanks.
@wmtan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.

@Dr15Jones
Copy link
Contributor Author

+1
The change to edm::AtomicPtrCache is from a merge with PR #1371

Reco update -- Import missing Winter13 BTag performance DB from CVS
@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2013

-1
I ran the usual tests and I found errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS
---> test runtestTqafTopEventSelection had ERRORS
---> test runtestTqafExamples had ERRORS
---> test runtestTqafTopJetCombination had ERRORS
---> test runtestTqafTopKinFitter had ERRORS
---> test runtestTqafTopHitFit had ERRORS
---> test runtestTqafTopTools had ERRORS
---> test runtestTqafTopEventProducers had ERRORS

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

ktf and others added 4 commits November 8, 2013 02:47
Multithreaded framework -- Added reset method to AtomicPtrCache
The non-const version of operator* was missing a ‘*’ and was not caught since it had not been called.
Updated the unit test so it tests all the functions, both const and non-const.
ROOT 5 complains about ROOT iorules for edm::AtomicPtrCache even though it is a transient data member. I spoke with Philippe Canal and he said it is a deficiency of Cintex and will not be in ROOT 6. This is a work around until we move to ROOT 6.
@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2013

Pull request #1375 was updated. @nclopezo, @cmsbuild, @Dr15Jones, @slava77, @vadler, @ktf, @thspeer can you please check and sign again.

@Dr15Jones
Copy link
Contributor Author

I had merged in PR #1387 and it now makes it look like I have 99 files changed. I'll try a rebase instead.

@Dr15Jones Dr15Jones closed this Nov 8, 2013
@Dr15Jones Dr15Jones deleted the threadSafeFixesDataFormatsPatCandidates branch November 8, 2013 15:15
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.

None yet

3 participants