Full version of EventSetup consumes system #26449
Conversation
Moved the implementations of the templated functions from EventSetupImp to EventSetup. These functions are only needed by the users of EventSetup, not EventSetupImpl. This sets up for further changes for EventSetup and records.
When using an ESGetToken, the EventSetup system makes sure that token was created for the given transition. A transition can be e.g. BeginRun or Event in the ED system or is a given Record in the ES system.
The information passed to the esConsumes call is now stored.
The class is used to hold information obtained from all EventSetup Records which is needed for the upcoming change to ESGetToken. Added methods to various EventSetup data structures to allow access to the needed information.
ESRecordsToProxyIndices is used by the modules to translate the ESGetToken indices to information needed to do the actual ES product lookup.
The test looks at the log to determine success. The log expects more than 100 messages to be given per category. This problem was only found when compiling with '-O0 -g'. Apparently there is bug in the message logger which causes the normal build to not properly enforce the limits.
The test assumed that after a sort, the container would be in a particular entry order. This is not actually guaranteed since different compiler optimization levels can change the sort order.
Instead of waiting for first synchronization, the configuration can be finalized explicitly.
The ESGetToken now just contains indices, not the strings from an ESInputTag. These indices are used to lookup the index for DataProxies in the appropriate EventSetup Record. The proxy index is then used to directly call the Proxy. The lookup tables from token index to proxy index are filled at begin job time.
|
The code-checks are being triggered in jenkins. |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26449/9254
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
The code-checks are being triggered in jenkins. |
|
Pull request #26449 was updated. @cmsbuild, @smuzaffar, @civanch, @Dr15Jones, @mdhildreth can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
+1 |
|
Comparison is ready Comparison Summary:
|
|
+1 |
cmsbuild
added
fully-signed
simulation-approved
and removed
pending-signatures
simulation-pending
labels
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
| @@ -152,19 +157,23 @@ namespace edm { | |||
| TReturn (T ::* iMethod)(const TRecord&), | |||
| const TArg& iDec, | |||
| const es::Label& iLabel = {}) { | |||
| const auto id = static_cast<unsigned int>(numberOfFactories()); | |||
There was a problem hiding this comment.
Possibly I am just not understanding something here. What does the numberOfFactories have to do with the transitionID getting passed into the Callback?
There was a problem hiding this comment.
Each factory generates a proxy for one and only one transition (i.e. EventSetup record). So the number of factories is just a way to identify which proxy is being called.
There was a problem hiding this comment.
What is worrying me is id is being passed into the constructor for the Callback immediately below this point (you have to scroll far to the right to see it in my browser). That is supposed to be a transition ID in the Callback constructor, not a Proxy identifier. Where id is used below that is probably good, but where it is passed into the Callback just seems incorrect. What if there are 10 DataProxy's? Then the value would be outside the range of the enum for transition IDs.
There was a problem hiding this comment.
One alternative would be to propagate the transition ID as an argument through all the various get* function calls in the stack that leads all the way back to the EventSetup get function call.
There was a problem hiding this comment.
@wddgit You are thinking that 'transitionID' here is of type 'edm::TransitionID'. It is not. For EDProducers the numeric value I use to keep track of 'transitionID' is the one from 'edm::TransitionID' since EDProducers have 'callbacks' (i.e. virtual functions) for only those transitions. For ESProducers, a 'transitionID' is an identifier for which 'setWhatProduced' is associated with which Callback. I.e. the value of 'transitionID' here is just local to that particular ESProducer. It is say which 'ESConsumesCollector' gathered the information for that given Callback.
|
+1 |
PR description:
The EventSetup consumes system now works with the ESGetToken to provide a more efficient data product lookup from a Record.
PR validation:
The code compiles and all framework related unit tests pass.