backport of #13448: small performance improvement in Tracking, and #14344: avoid redundant computations in StripCPE

[fwyzard]

backport to 8.0.x:

• Small performance improvement in Tracking #13448: Small performance improvement in Tracking
• avoid redundant computations in StripCPE #14344: avoid redundant computations in StripCPE

[fwyzard]

please test

[fwyzard]

tracked at #15151

[cmsbuild]

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

[cmsbuild]

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

It involves the following packages:

DataFormats/Common
DataFormats/SiStripCluster
DataFormats/SiStripDetId
RecoLocalTracker/ClusterParameterEstimator
RecoLocalTracker/SiPixelRecHits
RecoLocalTracker/SiStripRecHitConverter
RecoTracker/MeasurementDet
TrackingTools/MaterialEffects

@smuzaffar, @civanch, @Dr15Jones, @cvuosalo, @mdhildreth, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @wmtan, @makortel, @forthommel, @yduhm, @GiacomoSguazzoni, @gbenelli, @wddgit, @VinInn, @nickmccoll, @bellan, @jlagram, @dkotlins, @mschrode, @rovere, @gpetruc, @OlivierBondu, @threus, @dgulhan 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

[fwyzard]

I have included #13448 because #14344 seems strongly coupled to it.

[slava77]

@fwyzard please use more readable title for this PR.

[cmsbuild]

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

[cmsbuild]

-1

Tested at: 335deac

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15295/14243/summary.html

I found follow errors while testing this PR

Failed tests: Build ClangBuild

• Build:

I found an error when building:

>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/plugins/RecHitPropagator.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/test/MeasurementTrackerUpdator.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/FinalTrackSelectors/plugins/TrackCollectionMerger.cc 
>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/FinalTrackSelectors/plugins/CosmicTrackSelector.cc 
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/plugins/TkStripMeasurementDet.cc: In member function 'void TkStripMeasurementDet::simpleRecHits(const TrajectoryStateOnSurface&, const MeasurementTrackerEvent&, std::vector&) const':
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/plugins/TkStripMeasurementDet.cc:216:20: error: 'value_type' is not a member of 'TkStripMeasurementDet::AClusters {aka DynArray}'
     unInitDynArray(AClusters::value_type,detSet.size(),clusters);
                    ^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/plugins/TkStripMeasurementDet.cc:216:56: error: 'clusters' was not declared in this scope
     unInitDynArray(AClusters::value_type,detSet.size(),clusters);
                                                        ^


• Clang:

I found a compilation error while trying to compile with clang:
I used this command:
scram b vclean && scram build -k -j 16 USER_CXXFLAGS='-fsyntax-only' COMPILER='llvm compile'

      result.push_back(std::move(std::make_shared(recHit)));
                       ^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/plugins/TkStripMeasurementDet.h:186:24: note: remove std::move call here
      result.push_back(std::move(std::make_shared(recHit)));
                       ^~~~~~~~~~                                         ~
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/plugins/TkStripMeasurementDet.cc:216:31: error: no member named 'value_type' in 'DynArray'
    unInitDynArray(AClusters::value_type,detSet.size(),clusters);
                   ~~~~~~~~~~~^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_0_X_2016-07-24-2300/src/RecoTracker/MeasurementDet/plugins/TkStripMeasurementDet.cc:216:56: error: use of undeclared identifier 'clusters'
    unInitDynArray(AClusters::value_type,detSet.size(),clusters);
                                                       ^


[cvuosalo]

Should the "-Ofast" compiler option be eliminated from this PR as was done for #15280? It produces marginal timing improvements, and it creates some unnecessary differences, though they are tiny. From the tests described above, the number of difference plots for 25202.0 are as follows.

                  Validation      DQM
No Ofast             506          21121
With Ofast           567          21822

[slava77]

@cvuosalo
considering some visible benefits from Ofast in terms of time, and essentially the same count of histogram with differences, I think that we can keep this PR as is.

[cvuosalo]

@cmsbuild please test
Previous Jenkins tests were incomplete

[cmsbuild]

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

[cvuosalo]

+1

For #15295 ba6640b

Performance improvements in tracking and strip CPE.

The code changes match those put into 81X by #13448 and #14344. Jenkins tests show no problems, but they are incomplete. Extended tests described above show numerous, tiny, insignificant differences, more jitter and fluctuation than real differences. CPU timing tests indicate a speed up of around 5% for affected modules.

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

[cmsbuild]

+1
Tested at: ba6640b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15295/14508/summary.html

[cmsbuild]

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

[cvuosalo]

New Jenkins tests show the numerous, tiny, insignificant differences that are expected for this PR; otherwise, they show no problems.

[davidlange6]

+1