Serialize calls to EventSetup modules #1036
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nously so must be atomic
This version uses a TBB task per Stream to process Events with a mutex used to protect the Source in a critical section. This is sufficient to test the thread safety of the framework internals.
The EventProcessor now uses the new C++11 std::exception_ptr, std::current_exception() and std:rethrow_exception() to capture a thrown exception in a thread processing an event and then transfer that exception to the main thread to be rethrown. We use an std::atomic to provide thread safe update of the std::exception_ptr member data in the case of multiple processing threads having an exception. In such a case, only one of the exceptions will be kept.
The SharedResourcesRegistry is the centralized place where resources are registered and where the framework can get SharedResourcesAcquirer instances. These two classes work together to allow safe sharing of non-thread safe resources between modules. A unit test is included to test the two classes.
… to interface In order to more easily accommodate unscheduled execution, I switched from using a mutex to a recursive_mutex. This will be less performant since the locks will be held longer than might be needed but it is easier to implement initially. A more performant way would be to release the locks temporarily while we are doing a ‘edm::Event::getBy*’ call.
Now uses a std::mutex to guarantee a module instance is only processing one event at a time. Also uses a edm::SharedResourcesAcquirer to get the legacy resource and therefore guarantee only one legacy module is running at a time.
…haredResource All ‘one’ style base classes now hold a SharedResourcesAcquirer and use a new virtual function ‘createAcquirer()’ which by default returns an empty SharedResourcesAcquirer instance. If the module declares the use of a ‘SharedResource’ then the ‘createAcquirer()’ method will be overridden and return an instance with the needed resources. To facility this, edm::one::imll::SharedResourcesUser had to be changed to a templated class.
The SharedResourcesUser class was just changed to a template. As with the other implementor types, this needs to be explicitly instantiated.
Use a global mutex to serialize access to all EventSetup modules and sources. This is overkill but guarantees that if any two modules/sources are sharing a non-thread safe resource we don’t have a problem. Changed two of the mutable to std::atomics since they are read/written outside of the critical section guarded by the mutex.
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_7_0_THREADED_X. Serialize calls to EventSetup modules It involves the following packages: FWCore/Framework @smuzaffar, @Dr15Jones, @ktf, @nclopezo can you please review it and eventually sign? Thanks. |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. |
|
Merging this without testing. We should probably discuss a policy about how to proceed for approval since we expect things to be broken here... |
ktf
added a commit
that referenced
this pull request
Oct 9, 2013
Multithreaded framework -- Serialize calls to EventSetup modules
epalencia
added a commit
to epalencia/cmssw
that referenced
this pull request
Aug 2, 2022
Minor changes to GMTMuons Dataformat to include physical variables
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For now, we use a global recursive mutex to guard access to all EventSetup modules and sources. Using one mutex allows us to deal with the case where modules/sources are sharing the same non-thread safe resource.
Reading cached values from the EventSetup does not require a lock and is instead made thread safe via the use of std::atomics.