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

backport of #13831: Speedup Gsf component merging, return components by const reference #15279

Closed
wants to merge 29 commits into from

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Jul 26, 2016

includes #15259 (backport of #13594)

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 26, 2016

please test

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 26, 2016

does not seem to give any improvements to the HLT cpu usage:

Performance for reference
average from 42 tests:
        throughput:   60.9 +/-  0.6 ev/s
        event time:  348.2 +/-  2.8 ms

Performance for backport_13594+13831
average from 42 tests:
        throughput:   60.7 +/-  0.6 ev/s
        event time:  348.0 +/-  2.8 ms

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 26, 2016

tracked at #15151

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 26, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14224/console

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

CommonTools/Utils
DataFormats/Math
RecoParticleFlow/PFTracking
RecoTracker/TrackProducer
RecoVertex/GaussianSumVertexFit
TrackingTools/GeomPropagators
TrackingTools/GsfTools
TrackingTools/GsfTracking
TrackingTools/KalmanUpdators
TrackingTools/TrajectoryState

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @mmarionncern, @battibass, @makortel, @abbiendi, @GiacomoSguazzoni, @rafaellopesdesa, @jhgoh, @lgray, @bellan, @HuguesBrun, @mschrode, @rovere, @cbernet, @gpetruc, @VinInn, @trocino, @dgulhan, @bachtis this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@makortel
Copy link
Contributor

Just a note that 47cf66d is already part of #15240 (as commit 7cf0c3b). Probably doesn't matter in practice, but I thought to write it down anyway.

@slava77
Copy link
Contributor

slava77 commented Jul 26, 2016

merge conflicts

@@ -1,3 +1,4 @@
<flags CXXFLAGS="-Ofast"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe remove this, hoping to remove changes in RECO products and simplify integration?
... I didn't look if anything else could change the RECO products in this PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a major component of the speedup...

Copy link
Contributor Author

@fwyzard fwyzard Jul 26, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 7/26/16 10:42 AM, Vincenzo Innocente wrote:

In TrackingTools/GsfTools/plugins/BuildFile.xml
#15279 (comment):

@@ -1,3 +1,4 @@
+

this is a major component of the speedup...

There is essentially no speedup based on tests in HLT with PU40 done by
Andrea.

(also, recall another thread we had on -Ofast in April: there is no
significant speedup from this flag
in CMSSW RECO application
at least on the ~ 3yo Intel and AMD hardware I have tested)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/cms-sw/cmssw/pull/15279/files/ba717540b0163453d6beeec0f7d2cf0fd87765b8#r72300077,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbmcKCPhLRfTExfF15PI8TYBW-m_Zks5qZkcYgaJpZM4JU68_.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in contradiction with my test of the original version of this PR for 810

@fwyzard
Copy link
Contributor Author

fwyzard commented Aug 1, 2016

rebased on top of CMSSW_8_0_16

@fwyzard
Copy link
Contributor Author

fwyzard commented Aug 1, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14300/console

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2016

Pull request #15279 was updated. @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 1, 2016

@slava77
Copy link
Contributor

slava77 commented Aug 11, 2016

-1
as discussed in #15151 (comment)

@fwyzard fwyzard closed this Aug 26, 2016
@fwyzard fwyzard deleted the backport_13594+13831_80x branch September 28, 2016 09:57
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

5 participants