EG Pixel Match Speed updates #22427
EG Pixel Match Speed updates #22427
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22427/3685 |
|
A new Pull Request was created by @Sam-Harper for master. It involves the following packages: RecoEgamma/EgammaElectronAlgos @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
| @@ -48,7 +49,7 @@ FreeTrajectoryState FTSFromVertexToPointFactory::get( MagneticField const & magF | |||
| auto pyNew = -sa*pxOld + ca*pyOld; | |||
| GlobalVector pNew(pxNew, pyNew, pz); | |||
|
|
|||
| GlobalTrajectoryParameters gp(xmeas, pNew, charge, & magField); | |||
| GlobalTrajectoryParameters gp(xmeas, pNew, charge, & magField, std::move(magFieldAtPoint)); | |||
There was a problem hiding this comment.
is move really needed at this point?
Did you see a time improvement here?
There was a problem hiding this comment.
no its not needed, I can back it out, I just figured lets send it a && and if the code can ever deal with it in the future it'll pick it up. Its not costly? Do not have a strong opinon
There was a problem hiding this comment.
fine to leave it here.
IIUC, the GlobalVector does not have a move constructor. So the move here likely does nothing and prevents potential future use of magFieldAtPoint below this line.
OTOH, everything is inlined and the story of copying likely doesn't matter.
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
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) |
cmsbuild
added
fully-signed
and removed
pending-signatures
reconstruction-pending
labels
|
+1 |
Dear All,
Calculuate the B-field at a point is expensive and this function needlessly does it twice because the GlobalTrajectoryParameters also calculates the magnetic field when its created:
https://github.com/Sam-Harper/cmssw/blob/3b6bd6fd98d97088bf612c71ae5e81c6d24b32e5/TrackingTools/TrajectoryParametrization/src/GlobalTrajectoryParameters.cc#L35-L37
This class already has an option to pass the already computed value into its constructor, therefore I fixed the function to do this. From tests, this trivial and safe change saved another 1.5% CPU at the HLT.
-0.281609 -0.08% 0.79 ms/ev -> 0.60 ms/ev hltEgammaElectronPixelSeedsUnseeded
-0.227384 -1.40% 16.88 ms/ev -> 13.43 ms/ev hltEgammaElectronPixelSeeds
@Martin-Grunewald