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] ~12 million allocations from MTDGeometricTimingDetESModule::produce() #46512

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

Comments

@makortel
Copy link
Contributor

makortel commented Oct 24, 2024

From #45854 (comment)

The MTDGeometricTimingDetESModule::produce() makes ~12 million memory allocations, of which nearly all come from string construction in DDNameFilter::accept() (IgProf profile)

bool accept(const DDExpandedView& ev) const final {
std::string currentName(ev.logicalPart().name().fullname());

The ev.logicalPart().name().fullname() returns a newly constructed string

/** Returns a string complete of the \a namespace and \a name separated by ":".
Most likely you want to use ns() and / or name() methods instead.
*/
const std::string fullname() const { return ns() + ":" + name(); }

Especially how the DDNameFilter is used

DDNameFilter filter;
filter.add("mtd:");
filter.add("btl:");
filter.add("etl:");

I'd suggest to compare the namespace and name parts of DDName separately to avoid the string construction.

@makortel
Copy link
Contributor Author

assign Geometry/MTDNumberingBuilder

@cmsbuild
Copy link
Contributor

New categories assigned: geometry,upgrade

@bsunanda,@civanch,@Dr15Jones,@kpedro88,@makortel,@mdhildreth,@Moanwar,@srimanob,@subirsarkar 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

@cms-sw/mtd-dpg-l2

@makortel
Copy link
Contributor Author

type performance-improvements

@makortel
Copy link
Contributor Author

Addressed in #46531

@makortel
Copy link
Contributor Author

makortel commented Nov 8, 2024

The PR was merged

@makortel makortel closed this as completed Nov 8, 2024
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

2 participants