[WIP] TClass::GetClass: no interpreter lookups for fundamental types #13341
Conversation
Sonatype Lift is retiringSonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console. |
|
Starting build on |
core/meta/src/TClass.cxx
Outdated
| std::string name_no_std = name; | ||
| TClassEdit::RemoveStd(name_no_std); | ||
| auto lot = gROOT->GetListOfTypes(); | ||
| if(lot->Contains(name_no_std.c_str())) |
There was a problem hiding this comment.
The list of type contains not only raw types but also typedefs to class/struct or anything else that points to the typedefs
root [1] using MyTObject = TObject;
root [2] gROOT->GetListOfTypes()->FindObject("MyTObject")
(TObject *) 0x60000173d290
We can still save some cycles in this case:
auto dt = (TDataType*)gROOT->GetListOfTypes()->FindObject(name);
if (dt) {
if (dt->GetType() >=0)
// We have a fundamental type (or a typedef to)
return nullptr;
else
// We have a non fundamental, we can still use the information
name = dt->GetFullTypeName();
// we could check here if there is any trailings stars or trailing paranthesis to excluded pointers and functions (non trailing star/paranthesis might be template parameter)
}
There was a problem hiding this comment.
thanks for the feedback! I am proposing a new version of the code, also considering the failing tests.
|
Starting build on |
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Failing tests: |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
|
Build failed on ROOT-performance-centos8-multicore/cxx17. Failing tests: |
|
Build failed on mac11/noimt. Failing tests: |
|
Build failed on mac12arm/cxx20. Failing tests: |
|
Build failed on windows10/cxx14. Failing tests: |
|
Starting build on |
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/soversion. Failing tests: |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
|
Build failed on windows10/default. Failing tests: |
|
Build failed on mac11/noimt. Failing tests: |
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Failing tests: |
|
Build failed on mac12arm/cxx20. Failing tests: |
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
This commit solves issue root-project#9029 and, more in general, avoid lookups and parsing in some cases. One of the principles of the TClass::GetClass method implementation is to avoid as much as possible. This commit adds yet another fence in TClass::GetClass, checking if the name in input is the name of a known fundamental type or typedef to it. In order to avoid code duplication, a routine previously available within the implementation of TClassEdit has been made available with a public API.
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
|
Hello @phsft-bot build just on ROOT-fedora37, ROOT-fedora38, ROOT-ubuntu2204 |
| TClassEdit::GetNormalizedName(normalizedName, name); | ||
| } | ||
| std::string name_no_std = name; | ||
| TClassEdit::RemoveStd(name_no_std); |
There was a problem hiding this comment.
What is the advantage of using this name_no_std rather than the normalizedName?
|
@phsft-bot build |
|
Starting build on |
|
@phsft-bot build , still too many missing containers... |
|
Starting build on |
|
Thanks for all the feedback. A more comprehensive solution to avoid spurious lookups triggered by TClass::GetClass will be proposed. |
4 participants
Add yet another fence in TClass::GetClass to avoid lookups and memory consumption. This PR aims to fix #9029. Give the sophisticated implementation of TClass::GetClass, perhaps it would be good to collect some feedback, especially by @pcanal .
This Pull request:
This PR avoids lookups and parsing in some cases.
One of the principles of the TClass::GetClass method implementation is to avoid as much as possible.
Changes or fixes:
This commit adds yet another fence in TClass::GetClass, checking if the name in input is the name of a known fundamental type or typedef to it.
In order to avoid code duplication, a routine previously available within the implementation of TClassEdit has been made available with a public API.
Checklist:
This PR fixes #9029