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
MTD geometry: replace string with string_view in MTDBaseNumber #36106
Conversation
…e using it accordingly
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36106/26604
|
please test |
A new Pull Request was created by @fabiocos (Fabio Cossutti) for master. It involves the following packages:
@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2af255/20506/summary.html Comparison SummarySummary:
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36106/26637
|
Pull request #36106 was updated. @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please check and sign again. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2af255/20535/summary.html Comparison SummarySummary:
|
+1 |
@AdrianoDee @srimanob any comment? |
+Upgrade Technical PR to improve performance. PR test runs fine with no change in DQM. |
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Following recent discussions triggered by simulation performance reports, this PR update the
MTDBaseNumber
class, moving it from the use ofstring
to the use ofstring_view
. This is possible since the object is just used locally to build aDetId
and the underlying source of strings (geometry volume names) is always available to the code while being invoked.All the classes using this tool are updated accordingly.
PR validation:
Unit tests pass, and the workflow 34634.0 runs successfully without any change in DQM output.
An igprof test on the GEN-SIM part of the workflow (10 events) show a reduction of the rank of the
getBaseNumber
method used insideMtdSD
(already a minor contribution anyway):Original
Updated
A similar approach could be used for
EcalBaseNumber
, which has a much higher impact on the GEN-SIM step performances.