New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
making EgammaHLTPixelMatchVarProducer thread safe #19410
Conversation
A new Pull Request was created by @Sam-Harper for master. It involves the following packages: RecoEgamma/EgammaHLTProducers @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
The tests are being triggered in jenkins. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
+1 |
Dear All,
Unfortunately for some reason I cant explain, I made EgammaHLTPixelMatchVarProducer a global module when creating it. And unfortunately it uses a TF1 which is not thread safe. My apologies for this and thank you @slava77 for the spot.
Here is the fix. I also compared the variable it produces as a global module and as a stream module. I ran the standard HLT setup with 4 cores on a machine with 12 cores to ensure it was actually multithreading (400% CPU usage was observed). The results are here:
https://sharper.web.cern.ch/sharper/cms/heep/2017/Jun22nd/pmS2PreAndPostFix.png
indicating the effect is thankfully very small (no difference observed in 100K Z->ee events) However it is extremely serious hence this quick simple patch while I continue to work on #19263
It would have been sooner but been swamped with p5 operations today.