-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Concurrently run beginStreams #35230
Concurrently run beginStreams #35230
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35230/25189
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
cfa296c
to
93daef6
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35230/25191
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages:
@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@smuzaffar Should we put this in for DEVEL first? |
that is what I was thinking |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c7c23c/18504/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
|
yes sounds reasonable. |
enable threading |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c7c23c/18515/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
|
The PR looks good to me. I'm a bit hesitant to give |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_1_DEVEL_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_1_X is complete. 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:
Have each stream's beginStream transition run in its own TBB task.
PR validation:
Code compiles. Framework unit tests pass.
NOTE: It could very well be that CMSSW modules are doing thread unsafe activities in beginStream. This change needs to be tests on a large sample of multi-threaded jobs.