-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Core: Avoid (indirect) memory hoarding in TPluginManager. #14200
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
Conversation
This was reported at art-framework-suite/art#142 and is due to the combination of (a) We do not generate dictionary for std::tuple instances (b) When TClass::GetClass is called it tries to load the dictonary until there is a full TClass object is in memory (c) The emulated std::tuple TClass are marked as 'not loaded' (d) Searching for the TClass for a templated class will cost memory (during the lookup of the instantiation). (e) TPluginManager::ExecPluginImpl was looking up the TClass for the typle `std::type< list of arguments>` The lookup induced in (e) in the user's case (root built with runtime cxx module on) lead to some memory allocation in Clang while trying to find out if there was now a library or dictionary to load.
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/soversion. Failing tests:
|
|
Starting build on |
|
The New CI failures seems unrelated. |
|
Patch was successfully back-ported to 6.30/02 and reported by Mu2e as resolving art-framework-suite/art#142 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these changes! LGTM. I am sure this will help in cases different from Mu2e.
|
It's hard to pin down, but looking through the commit history and its incremental builds it seems that this PR made FYI @bellenot |
|
I confirm that these changes break |
This was reported at art-framework-suite/art#142 and is due to the combination of
(a) We do not generate dictionary for std::tuple instances (b) When TClass::GetClass is called it tries to load the dictonary until there is a full TClass object is in memory (c) The emulated std::tuple TClass are marked as 'not loaded' (d) Searching for the TClass for a templated class will cost memory (during the lookup of the instantiation). (e) TPluginManager::ExecPluginImpl was looking up the TClass for the typle
std::type< list of arguments>The lookup induced in (e) in the user's case (root built with runtime cxx module on) lead to some memory allocation in Clang while trying to find out if there was now a library or dictionary to load.
This fixes #14199