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

HLTPrescaleProvider optimization: minimize repeated calls to retrieveL1Event #19085

Closed
slava77 opened this issue Jun 2, 2017 · 6 comments
Closed

Comments

@slava77
Copy link
Contributor

slava77 commented Jun 2, 2017

Sub-optimal behavior was found in analysis ntupling code.
The code loops over all triggers in the table (per event) and calls HLTPrescaleProvider::prescaleValuesInDetail, which turned out to be a significant user of CPU.

Profiler showed that more than 95% of CPU is taken by calls to l1t::L1TGlobalUtil::retrieveL1Event repeated twice for every call to HLTPrescaleProvider::prescaleValuesInDetail. One is called directly, and another via ::prescaleSet method call.
The retrieveL1Event call is the same per event, while the call to prescaleValuesInDetail is specific per trigger.

Calls to l1t::L1TGlobalUtil::retrieveL1Event can be done once per event with appropriate caching.

@slava77
Copy link
Contributor Author

slava77 commented Jun 2, 2017

assign hlt

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 2017

New categories assigned: hlt

@Martin-Grunewald,@silviodonato,@fwyzard you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 2017

A new Issue was created by @slava77 Slava Krutelyov.

@davidlange6, @Dr15Jones, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Jun 21, 2017

It looks to me one could simply remove the calls to retrieveL1Event (case l1tType==2) here:
https://github.com/cms-sw/cmssw/blob/master/HLTrigger/HLTcore/src/HLTPrescaleProvider.cc#L163
(method prescaleValues) and here:
https://github.com/cms-sw/cmssw/blob/master/HLTrigger/HLTcore/src/HLTPrescaleProvider.cc#L285
(method prescaleValuesInDetail) because method prescaleSet is called beforehand within each method and that one calls retrieveL1Event already.

I'd just hope that MT would not screw this up!

@Dr15Jones ?

@Dr15Jones
Copy link
Contributor

@Martin-Grunewald the HLTPrescaleProvider is already thread-friendly but not thread safe so as long as each module has its own copy of the object, there shouldn't be a threading problem.

This was referenced Jul 6, 2017
@Martin-Grunewald
Copy link
Contributor

PRs made - can close this issue!

@slava77 slava77 closed this as completed Mar 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants