Implement global::OutputModuleBase #11330
Conversation
|
A new Pull Request was created by @wmtan for CMSSW_7_6_X. Implement global::OutputModuleBase It involves the following packages: FWCore/Framework @cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
| } | ||
| if(remainingEvents_ > 0) { | ||
| --remainingEvents_; | ||
| } |
There was a problem hiding this comment.
This isn't thread safe. remainingEvents_ could have changed from >0 to 0 between the two calls. Probably the following is better
auto remainingEvents = remainingEvents_.load();
bool keepTrying = remainingEvents > 0;
while (keepTrying){
auto newValue = remainingEvents -1;
keepTrying = not remainingEvents_.compare_exchange_strong(remainingEvents, newValue);
if( keepTrying) {
//the exchange failed because the value was changed by another thread.
// remainingEvents was changed to be the new value of remainingEvents_;
keepTrying = remainingEvents >0;
}
}
wmtan
force-pushed
the
GlobalOutputModule
branch
from
0a14bf4
to
7ea6755
Compare
|
@Dr15Jones Both of your comments have been adopted. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Pull request #11330 was updated. @cmsbuild, @smuzaffar, @Dr15Jones can you please check and sign again. |
|
+1 |
cmsbuild
added
core-approved
fully-signed
and removed
core-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
@smuzaffar is this test still running? |
|
@smuzaffar @davidlange6 The build timed out since so much of the release must be built. This really is just adding in a new type which isn't used by others but had to touch a file that is used by almost everything. I'd vote to just add this directly to a release. |
|
@Dr15Jones, I have restarted the test. We also noticed that same issue with #11427 and notice that in Pr test we are compiling three times all the packages (packages part of PR and package checked out by checkdeps)
I now have changed the logic and first two compilations are only done for the packages which are effected by the PR only. I also have increased the jenkins job time to be 8 hour instead of 5. |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
+1 |
Implement global::OutputModuleBase
This PR implements a base class to support global output modules. It also changes AsciiOutputModule to be global, and adds an explicit unit test for global output modules.