Implement global::OutputModuleBase

[wmtan]

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.

[cmsbuild]

A new Pull Request was created by @wmtan for CMSSW_7_6_X.

Implement global::OutputModuleBase

It involves the following packages:

FWCore/Framework
FWCore/Modules
FWCore/ParameterSet

@cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit 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.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[wmtan]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.

[Dr15Jones]

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]

Done.

[wmtan]

@Dr15Jones Both of your comments have been adopted.

[wmtan]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.

[cmsbuild]

Pull request #11330 was updated. @cmsbuild, @smuzaffar, @Dr15Jones can you please check and sign again.

[Dr15Jones]

+1

[cmsbuild]

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

[Dr15Jones]

@smuzaffar is this test still running?

[Dr15Jones]

@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.

[smuzaffar]

@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)

1. clang compilation check
2. llvm static analyzer
3. Normal scram build

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.

[Dr15Jones]

It looks like #11441 and #11469 are also affected

[cmsbuild]

+1
Tested at: 7ea6755
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11330/8388/summary.html

[cmsbuild]

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

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11330/8388/summary.html

[cmsbuild]

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

[davidlange6]

+1