Avoid unneccessary copies in RecoParticleFlow__PFProducer with emplace

[guitargeek]

No description provided.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24846/6806

[cmsbuild]

A new Pull Request was created by @guitargeek (Jonas Rembser) for master.

It involves the following packages:

RecoParticleFlow/PFProducer

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @lgray, @seemasharmafnal, @bachtis, @cbernet this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[slava77]

@guitargeek
did you see any change in performance?
Most of your changes look like the compiler will do already and will yield a no-op from this PR.

[guitargeek]

Hi Slava, no I didn't check performance. Sorry, I suddenly had no time anymore to write a good description.

So here is the thing: working on the EGamma PF algo to e.g. improve it for low-pt is not so easy because the code is hard to digest. One of my little personal goals is therefore to refactor PFEGammaAlgo to make it more comprehensible such that is can be easily hacked with and improved.

In the past I made some experience with my "mammoth" PRs, which I think might took just more time to review than if there were split up in little pieces because they touched many aspects at once.

So yes, this PR does nothing except for making the code (in my opinion) more expressive. Why counting on compiler optimizations if you can say with even fewer words how it should be?

So: this PR would be just a tiny step towards a bigger goal. It's up to you to decide if that makes sense or if it would be just a waste of effort :)

[slava77]

@guitargeek
thanks for the longer message.

I was just trying to understand if "avoid unnecessary copies" was a factual thing at run time.
If it is the case, it can be a reason to review similar cases in our code base. Otherwise, it has less incentive to be applied elsewhere.

I'm fine if this is just a cosmetic update.

[slava77]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31000/console Started: 2018/10/12 10:22

[cmsbuild]

+1
Tested at: 034f525
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24846/31000/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 32
• DQMHistoTests: Total histograms compared: 2995443
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2995245
• DQMHistoTests: Total skipped: 197
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
• Checked 134 log files, 14 edm output root files, 32 DQM output files

[slava77]

+1

for #24846 034f525

• technical update; there is no clear evidence that this introduces reduction in CPU use or memory allocations. Still, the style is improved.
• jenkins tests pass and comparisons with the baseline show no differences

[cmsbuild]

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)

[fabiocos]

+1