migrate few E/Gamma HLT modules from legacy to stream or global

[fwyzard]

No description provided.

[cmsbuild]

A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_7_3_X.

migrate few E/Gamma HLT modules from legacy to stream or global

It involves the following packages:

RecoEgamma/EgammaHLTProducers

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

[Martin-Grunewald]

Hi @fwyzard

I am confused how you decide between "stream" and "global".
I thought that "stream" should be default (for EDProducers and EDFilters,
who are fine with just seeing the stream-subset of events) while "global"
should just be for those which need to see all events, such as EDAnalyzers
or OMs.
So what drives your choices?

[fwyzard]

Hi Martin,
so far I'm basing my choice on this:

• if the module does not have any non-const data members (i.e., nothing but the configuration parameters), I make it global ;
• if the module has non-const data members (including objects that depend on the current IOV), I make it stream .

My assumption is that a global module is slightly more efficient, so I try that when possible.

.A

[fwyzard]

By the way, we should add some multithreading run-time tests.
@Martin-Grunewald , is it OK for you if I turn on multithreading in the pass of hltIntegrationTest that runs the whole menu ?
Comparing its results with single-threaded, single-trigger reseults should spot some basic problems.

[Martin-Grunewald]

Please make it a configurable option with some default... :)

[Dr15Jones]

global modules should be faster (probably small) and use less memory

[Dr15Jones]

Are you also looking at the static analyzer report to be sure those modules do not access any global variables?

[fwyzard]

hi @Dr15Jones ,
actually no, I've just gone quickly through the code... I'll check that, too.

[Dr15Jones]

You'll want to look at 'Modules to thread unsafe statics' off of the Integration build page.
http://cms-sw.github.io/showIB.html

[cmsbuild]

+1
Tested at: b5e1be4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6449/952/summary.html

[cmsbuild]

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

[fwyzard]

@Martin-Grunewald : the parallel checks are implemented in #6461 .

[Martin-Grunewald]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes (tests are also fine). This pull request will be automatically merged.