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

Made use of edm::RefCore thread-safe #9114

Merged
merged 7 commits into from May 17, 2015

Conversation

Dr15Jones
Copy link
Contributor

The internal caching done by an edm::RefCore is now thread safe. All uses of edm::RefCore were also changed to be thread-safe.

Dr15Jones and others added 7 commits May 14, 2015 13:02
…cache

The cachePtr_ is used to hold either an EDProductGetter or the results of getting data from the getter. Previously the state was encoded in one of the bits in ProcessIndex. However, that structure makes it difficult to change RefCore to be thread safe. Consolidating the address and the state into one memory address makes it possible to use an std::atomic.
This design makes the assumption that the RefCore must be aligned on at least an even byte boundary. This is definitely true for all the systems we use since the 64 bit pointer requires that the class be aligned on a 64 bit boundary.
Made how edm::RefCore switches its cached value from an EDProductGetter to the value obtained from the getter to be thread safe via an atomic update.
Updated all DataFormats/Common classes which use edm::RefCore to also be thread safe on cache changes.
There was a bug in ROOT 6 which was requiring a dictionary be generated
for a class which was only used as a transient data member. The bug
was fixed so the workaround can be removed.��
Made the necessary code changes to be able to compile using ROOT 5.
The RefCore code was changed to be thread-safe. This meant that calling
productPtr() could change its answer. This change properly handles
that possibility.
Add a unit test for edm::Ref which uses multiple threads to dereference the same edm::Ref.
@cmsbuild
Copy link
Contributor

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

Made use of edm::RefCore thread-safe

It involves the following packages:

DataFormats/Common
DataFormats/TrackReco

@nclopezo, @cvuosalo, @cmsbuild, @Dr15Jones, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @gpetruc, @VinInn, @wddgit, @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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@Dr15Jones
Copy link
Contributor Author

please test

@Dr15Jones
Copy link
Contributor Author

+1

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@slava77
Copy link
Contributor

slava77 commented May 16, 2015

+1

for #9114 2511d40

  • changes in the code look OK
  • jenkins tests pass and show no differences both in the fwlite and DQM monitoring comparisons with the baseline

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @nclopezo, @smuzaffar

@davidlange6
Copy link
Contributor

+1

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

4 participants