Modernize L1Trigger/CSCTrackFinder modules #25305
Conversation
Holding by value is more efficient and keeps const correctness.
-converted to a global module -hold CSCTFCandidateBuilder by value to keep const correctness -use EDPutToken
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25305/7332 |
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages: L1Trigger/CSCTrackFinder @nsmith-, @rekovic, @cmsbuild, @thomreis can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
-Made it a one module since it depends on thread-hostile code -Used std::unique_ptr -Avoided the use of pointer when by value would work -Made member variables const when appropriate
cmsbuild
added
code-checks-pending
tests-pending
and removed
code-checks-approved
tests-started
labels
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25305/7335 |
Dr15Jones
changed the title
|
please test |
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-notrun
tests-rejected
labels
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
Changes look OK but the static checks find lots of potential thread unsafe static variables. Should they be addressed? |
|
Those statics are why I had to make CSCTFTrackProducer a one module. I believe when I've asked in the passed I was told some of those statics are auto generated from a firmware tool and the code it creates is full of globals. |
|
@thomreis I'm not saying I do not think the statics should be left as they are forever. It is just such changes are beyond the scope of what I'm trying to accomplish. |
|
+1 Thanks @Dr15Jones Just wanted to be sure those have been considered. |
cmsbuild
added
fully-signed
l1-approved
and removed
l1-pending
pending-signatures
labels
|
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) |
|
+1 |
Made CSCTFCandidateProducer a global module since it was causing synchronization on LuminosityBlock boundaries in the DIGI workflows.
-Improved const correctness by holding data members by value instead of pointer
-Switched to using EDPutToken
Made CSCTFTrackProducer a one module since it calls thread-hostile code.