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

use std::shared_ptr and std::make_shared (EventSetup in Reco) #13898

Merged
merged 1 commit into from Apr 4, 2016

Conversation

wmtan
Copy link
Contributor

@wmtan wmtan commented Apr 2, 2016

The only place that the CMS framework still uses boost::shared_ptr is in the interface to the EventSetup system, where std::shared_ptr is also supported. In order to remove this last vestige of boost::shared_ptr,
users of EventSetup need to be converted to std::shared_ptr. This PR does this for Reco packages.
Also, std::make_shared is implemented where it makes sense, because it simplifies the code and saves a memory allocation.
Packages TrackingTools/Gsf* are not done in this PR because there is another PR from Vincenzo Innocente making some of the same changes, and I wish to avoid a confluct with that PR.
This PR is totally technical.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 2, 2016

A new Pull Request was created by @wmtan for CMSSW_8_1_X.

It involves the following packages:

EventFilter/EcalRawToDigi
RecoBTag/PerformanceDB
RecoBTau/JetTagComputer
RecoEcal/EgammaCoreTools
RecoEgamma/ElectronIdentification
RecoLocalCalo/EcalRecAlgos
RecoLocalCalo/EcalRecProducers
RecoLocalCalo/HcalRecAlgos
RecoLocalMuon/RPCRecHit
RecoLocalTracker/Phase2TrackerRecHits
RecoLocalTracker/SiPixelRecHits
RecoLocalTracker/SiStripClusterizer
RecoLocalTracker/SiStripRecHitConverter
RecoLuminosity/LumiProducer
RecoMuon/DetLayers
RecoMuon/TransientTrackingRecHit
RecoPixelVertexing/PixelLowPtUtilities
RecoTracker/GeometryESProducer
RecoTracker/MeasurementDet
RecoTracker/SiTrackerMRHTools
RecoTracker/TkNavigation
RecoTracker/TransientTrackingRecHit
RecoVertex/BeamSpotProducer
TrackingTools/KalmanUpdators
TrackingTools/MaterialEffects
TrackingTools/PatternTools
TrackingTools/Producers
TrackingTools/RecoGeometry
TrackingTools/TrackAssociator
TrackingTools/TrackFitters
TrackingTools/TransientTrack

@cvuosalo, @cerminar, @cmsbuild, @franzoni, @slava77, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @forthommel, @yduhm, @argiro, @nickmccoll, @threus, @battibass, @makortel, @acaudron, @jhgoh, @lgray, @jlagram, @ferencek, @OlivierBondu, @trocino, @rociovilar, @Sam-Harper, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @tocheng, @cerati, @mschrode, @dgulhan, @Martin-Grunewald, @abbiendi, @imarches, @gbenelli, @dkotlins, @gpetruc, @istaslis, @pvmulder, @bachtis this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@wmtan
Copy link
Contributor Author

wmtan commented Apr 2, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 2, 2016

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

@VinInn
Copy link
Contributor

VinInn commented Apr 2, 2016

@wmtan are you planning to migrate all other usage of boost to std?
there is a boost::shared_ptr<const TrajectorySeed> in Trajectory that I was going to migrate as part of some other algorithmic changes. If you are already on it I will not touch it.

@wmtan
Copy link
Contributor Author

wmtan commented Apr 2, 2016

@VinInn No, I was just migrating those uses that were needed for removing support for boost::shared_ptr in EventSetup. This PR does all of those in Reco except for those in TrackingTools/Gsf*. I did not do those because I saw that your existing PR touches some of the same lines.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 2, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 2, 2016

@VinInn
Copy link
Contributor

VinInn commented Apr 3, 2016

Please merge all these PRs involving shared_ptr asap: they touch almost everything, difficult to develop w/o risking conflicts...

@mmusich
Copy link
Contributor

mmusich commented Apr 4, 2016

+1
alca involved only through minor changes in RecoVertex/BeamSpotProducer

@cvuosalo
Copy link
Contributor

cvuosalo commented Apr 4, 2016

+1

For #13898 b7ba4b0

Purely technical change to eliminate boost::shared_ptr, replacing it with std:shared_ptr. There should be no change in monitored quantities.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-04-02-1100 show no significant differences, as expected.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2016

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 6eb160c into cms-sw:CMSSW_8_1_X Apr 4, 2016
@wmtan wmtan deleted the StdSharedPtrInReco branch May 26, 2016 04:35
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