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

[Phase2 RelVal] O(10 million) memory allocations per event by Phase2TrackerMonitorDigi::analyze() #46510

Closed
makortel opened this issue Oct 24, 2024 · 9 comments

Comments

@makortel
Copy link
Contributor

From #45854 (comment)

In Phase2 RelVal Reco job the Phase2TrackerMonitorDigi::analyze() seems to make O(10 million) memory allocations per event. According to the IgProf profile nearly all of these come from MessageLogger.

I see several edm::LogInfo("Phase2TrackerMonitorDigi") << ...; calls there. I guess what happens is that the Phase2TrackerMonitorDigi formats the Info-level messages and injects them into the MessageLogger queue, and MessageLogger then discards the messages because the default logging level is above Info.

I'd suggest to either change the most frequent to LogDebug, in which case the message formatting will be compiled away by default, or change the logging pattern to

edm::LogInfo("Phase2TrackerMonitorDigi").log([&](auto& l) {
  l << ...l;
});

in which case the lambda passed to log() will be called only if the logging level is at least Info (i.e. with the default MessageLogger configuration the lambda will not be called).

@makortel
Copy link
Contributor Author

assign DQM/SiTrackerPhase2

@makortel
Copy link
Contributor Author

type performance-improvements

@cmsbuild
Copy link
Contributor

New categories assigned: dqm

@antoniovagnerini,@nothingface0,@rvenditti,@syuvivida,@tjavaid you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 24, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @makortel.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor Author

assign upgrade

@cmsbuild
Copy link
Contributor

New categories assigned: upgrade

@Moanwar,@srimanob,@subirsarkar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@mmusich
Copy link
Contributor

mmusich commented Oct 31, 2024

I'd suggest to either change the most frequent to LogDebug

I think there's no real reason to keep any of the LogInfo-s.
I did that #46575 (among other things)

@makortel
Copy link
Contributor Author

I did that #46575 (among other things)

The PR was merged, so I think we can close the issue

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

3 participants