Skip to content
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

Optimization of PixelHitMatcher #3020

Merged
merged 1 commit into from Mar 29, 2014
Merged

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Mar 25, 2014

Factor of 3.5 speed up in ecalDrivenElectronSeeds producer.
4x speed up in compatibleSeeds() function in PixelHitMatcher class.
Mostly this comes from using hash tables, some speedup from not using GlobalPoint::phi() all the time and lazy returns.
Timing measurements are from TTBar 25ns@40PU.

No regressions expected.
In local testing of single electron gun and ttbar 25ns, no regressions observed.

@VinInn

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_7_1_X.

Optimization of PixelHitMatcher

It involves the following packages:

RecoEgamma/EgammaElectronAlgos

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
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, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Mar 28, 2014

checking

@slava77
Copy link
Contributor

slava77 commented Mar 29, 2014

Looking at 202.0 (2012-like TTBar PU) on 200 events with igprof I see only 10% improvement from PixelHitMatcher::compatibleSeeds: going from 15 perf ticks to 13.6.
Most notable part here is that inside this function the call to PropagatorWithMaterial::propagateWithPath has doubled in weight from 4 to 8 ticks.

@VinInn
Copy link
Contributor

VinInn commented Mar 29, 2014

at pileup 40 25ns is very different
this is the a recent IB for TTBAR PU40 25ns
http://innocent.web.cern.ch/innocent/perfResults/igprof-navigator/TTBAR_step3_ori_CMSSW_7_1_X_2014-03-14-0200_slc6_amd64_gcc490/103
notice rank 103
(compare with yours)

this is jetHT run2012C
http://innocent.web.cern.ch/innocent/perfResults/igprof-navigator/step2_Run2012C_Candidate_CMSSW_7_1_X_2014-03-28-0200_slc6_amd64_gcc481/365
(should be similar to yours)
here is at rank 365

very very different contributions to
PixelHitMatcher::compatibleSeeds
in the two cases

@slava77
Copy link
Contributor

slava77 commented Mar 29, 2014

I see, so ::estimate vs ::propagate calls are quite different in relative weight.
Other that ::propagate, the total of all other calls inside PixelHitMatcher::compatibleSeeds goes down by x2 in my wf 202.0. I can understand that this goes up with higher occupancy.
Still, an increase in calls to ::propagate is strange.

@slava77
Copy link
Contributor

slava77 commented Mar 29, 2014

+1

for #3020 6760ed8
tested in CMSSW_7_1_X_2014-03-25-1400 (my test area sign329)

no differences in outputs, as expected.
The summary/discussion for timing is above. the increase in ::propagate looks odd, could be fixed later, I suppose, if it's a real.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

ktf added a commit that referenced this pull request Mar 29, 2014
Reco -- Optimization of PixelHitMatcher
@ktf ktf merged commit cceeb18 into cms-sw:CMSSW_7_1_X Mar 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants