From a04ff4cc71cede456ed08275df0dd551236cc853 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Thu, 3 Sep 2026 12:52:04 +0000 Subject: [PATCH] [core] Reuse the CheckClassInfo lookup result in TClass::Init() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TClass::Init() first calls TCling::CheckClassInfo() (which looks up the class with cling's LookupHelper::findScope()) and then TCling::SetClassInfo(), which repeated the exact same lookup to construct the TClingClassInfo it stores in the TClass. CheckClassInfo() now optionally returns the class info it found to the caller, and SetClassInfo() optionally accepts one, taking its ownership, instead of looking the class up again. TClass::Init() passes it through. The result is only handed out when findScope() itself returned a declaration. findScope() only does so when the declaration points to an already complete definition (or a namespace), in which case its instantiateTemplate argument has no effect on the result; when the lookup only produced a forward-declared template specialization, findScope() returns a null declaration together with a non-null type, CheckClassInfo() hands nothing back, and SetClassInfo() performs the template-instantiating lookup exactly as before. The found type is passed along with the declaration so that typedef sugar (e.g. Double32_t) is conserved just like in the name-based lookup. SetClassInfo() ignores a provided class info when 'reload' is set (a reload must redo the lookup) and in the tuple<...> special case, which overlays an alternate implementation with a different name. The InsertStd() retry in CheckClassInfo() now collects the result type into a separate variable: findScope() does not write it on every path (e.g. when finding a namespace), so the retry could otherwise pair its declaration with a stale type left over from the first lookup. This removes all duplicated findScope() calls during startup of root.exe: 206 -> 185 lookups, of which 9 were the TClingClassInfo constructor repeating a CheckClassInfo() lookup. Fixes #7123 🤖 Done with the help of AI --- core/meta/inc/TInterpreter.h | 10 ++++- core/meta/src/TClass.cxx | 9 +++- core/metacling/src/TCling.cxx | 57 +++++++++++++++++++++----- core/metacling/src/TCling.h | 12 ++++-- core/metacling/src/TClingClassInfo.cxx | 15 +++++-- core/metacling/src/TClingClassInfo.h | 2 +- 6 files changed, 83 insertions(+), 22 deletions(-) diff --git a/core/meta/inc/TInterpreter.h b/core/meta/inc/TInterpreter.h index 436406f72eaf1..fe782d13421ec 100644 --- a/core/meta/inc/TInterpreter.h +++ b/core/meta/inc/TInterpreter.h @@ -210,14 +210,20 @@ class TInterpreter : public TNamed { virtual void UpdateListOfGlobals() = 0; virtual void UpdateListOfGlobalFunctions() = 0; virtual void UpdateListOfTypes() = 0; - virtual void SetClassInfo(TClass *cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE) = 0; + virtual void SetClassInfo(TClass *cl, + Bool_t reload = kFALSE, + Bool_t silent = kFALSE, + ClassInfo_t *classInfo = nullptr) = 0; enum ECheckClassInfo { kUnknown = 0, // backward compatible with false kKnown = 1, kWithClassDefInline = 2 }; - virtual ECheckClassInfo CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamespaceOnly = kFALSE) = 0; + virtual ECheckClassInfo CheckClassInfo(const char *name, + Bool_t autoload, + Bool_t isClassOrNamespaceOnly = kFALSE, + ClassInfo_t **classInfo = nullptr) = 0; virtual Bool_t CheckClassTemplate(const char *name) = 0; virtual Longptr_t Calc(const char *line, EErrorCode* error = nullptr) = 0; diff --git a/core/meta/src/TClass.cxx b/core/meta/src/TClass.cxx index 37c2b3824ae2c..f9d896d5c18cc 100644 --- a/core/meta/src/TClass.cxx +++ b/core/meta/src/TClass.cxx @@ -1483,8 +1483,13 @@ void TClass::Init(const char *name, Version_t cversion, if (proto) proto->FillTClass(this); } - if (!fHasRootPcmInfo && gInterpreter->CheckClassInfo(fName, /* autoload = */ kTRUE)) { - gInterpreter->SetClassInfo(this, kFALSE, silent); // sets fClassInfo pointer + ClassInfo_t *checkedInfo = nullptr; + if (!fHasRootPcmInfo && + gInterpreter->CheckClassInfo(fName, /* autoload = */ kTRUE, /* isClassOrNamespaceOnly = */ kFALSE, + /* classInfo = */ &checkedInfo)) { + // Pass along the class info that CheckClassInfo may have found, so that + // SetClassInfo (which takes its ownership) does not repeat the lookup. + gInterpreter->SetClassInfo(this, kFALSE, silent, checkedInfo); // sets fClassInfo pointer if (fClassInfo) { // This should be moved out of GetCheckSum itself however the last time // we tried this cause problem, in particular in the end-of-process operation. diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index ac7400214fa20..32c8187c59694 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -4205,9 +4205,20 @@ static std::string AlternateTuple(const char *classname, const cling::LookupHelp /// Set pointer to the TClingClassInfo in TClass. /// If 'reload' is true, (attempt to) generate a new ClassInfo even if we /// already have one. +/// If 'classInfo' is non-null, take ownership of it and use it instead of +/// looking up the class again; it must have been obtained from a call to +/// CheckClassInfo(cl->GetName(), ...). -void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent) +void TCling::SetClassInfo(TClass *cl, Bool_t reload, Bool_t silent, ClassInfo_t *classInfo) { + // Whether we use it below or not, we own the passed class info. + std::unique_ptr providedInfo{(TClingClassInfo *)classInfo}; + + // A provided class info is a cached lookup result; honoring it would defeat + // the point of a reload, which is to redo the lookup. + if (reload) + providedInfo.reset(); + // We are shutting down, there is no point in reloading, it only triggers // redundant deserializations. if (fIsShuttingDown) { @@ -4252,6 +4263,9 @@ void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent) // details and just overlay a 'simpler'/'simplistic' version that is easy // for the I/O to understand and handle. if (strncmp(cl->GetName(),"tuple<",std::char_traits::length("tuple<"))==0) { + // A provided class info would describe the real std::tuple, not the + // alternate version overlaid below: it cannot be used. + providedInfo.reset(); if (!reload) name = AlternateTuple(cl->GetName(), fInterpreter->getLookupHelper(), silent); if (reload || name.empty()) { @@ -4266,7 +4280,8 @@ void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent) // that is currently in the caller (like SetUnloaded) that disable AutoLoading and AutoParsing and // code is in the callee (disabling template instantiation) and end up with a more explicit class: // TClingClassInfoReadOnly. - TClingClassInfo* info = new TClingClassInfo(GetInterpreterImpl(), name.c_str(), instantiateTemplate); + TClingClassInfo *info = providedInfo ? providedInfo.release() + : new TClingClassInfo(GetInterpreterImpl(), name.c_str(), instantiateTemplate); if (!info->IsValid()) { SetWithoutClassInfoState(cl); delete info; @@ -4340,11 +4355,20 @@ void TCling::SetClassInfo(TClass* cl, Bool_t reload, Bool_t silent) /// specifically check that each level of nesting is already loaded. /// In case of templates the idea is that everything between the outer /// '<' and '>' has to be skipped, e.g.: `aap::klaas>::a_class` +/// +/// If 'classInfo' is non-null and the lookup found a declaration (which +/// findScope only returns if it points to a complete definition, i.e. when no +/// template instantiation would be needed to create it), '*classInfo' is set +/// to a newly allocated TClingClassInfo for that declaration, owned by the +/// caller. Passing it to SetClassInfo() avoids repeating the lookup there. -TInterpreter::ECheckClassInfo -TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamespaceOnly /* = kFALSE*/) +TInterpreter::ECheckClassInfo TCling::CheckClassInfo(const char *name, Bool_t autoload, + Bool_t isClassOrNamespaceOnly /* = kFALSE*/, + ClassInfo_t **classInfo /* = nullptr*/) { R__LOCKGUARD(gInterpreterMutex); + if (classInfo) + *classInfo = nullptr; static const char *anonEnum = "anonymous enum "; static const int cmplen = strlen(anonEnum); @@ -4416,13 +4440,24 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp : cling::LookupHelper::NoDiagnostics, &type, /* intantiateTemplate= */ false ); if (!decl) { + // Use a separate output type for the retry: findScope does not write it + // on every path (e.g. when finding a namespace), and the type left over + // from the lookup above must not be paired with this lookup's decl. + const clang::Type *typeFromStd = nullptr; std::string buf = TClassEdit::InsertStd(classname); - decl = lh.findScope(buf, - gDebug > 5 ? cling::LookupHelper::WithDiagnostics - : cling::LookupHelper::NoDiagnostics, - &type,false); + decl = lh.findScope(buf, gDebug > 5 ? cling::LookupHelper::WithDiagnostics : cling::LookupHelper::NoDiagnostics, + &typeFromStd, false); + if (decl || typeFromStd) + type = typeFromStd; } + // If requested and an entity was found by the lookup above, hand a class + // info for it out to the caller (see the function documentation). + auto provideClassInfo = [this, classInfo, &decl, &type] { + if (classInfo && decl && !decl->isInvalidDecl()) + *classInfo = (ClassInfo_t *)new TClingClassInfo(GetInterpreterImpl(), decl, type); + }; + if (type) { // If decl==0 and the type is valid, then we have a forward declaration. if (!decl) { @@ -4477,6 +4512,7 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp // , hasClassDefInline); // We are now sure that the entry is not in fact an autoload entry. + provideClassInfo(); if (hasClassDefInline) return kWithClassDefInline; else @@ -4487,9 +4523,10 @@ TCling::CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamesp } } - if (decl) + if (decl) { + provideClassInfo(); return kKnown; - else + } else return kUnknown; // Setting up iterator part of TClingTypedefInfo is too slow. diff --git a/core/metacling/src/TCling.h b/core/metacling/src/TCling.h index 66e32f6502516..1c2be1bace445 100644 --- a/core/metacling/src/TCling.h +++ b/core/metacling/src/TCling.h @@ -277,9 +277,15 @@ class TCling final : public TInterpreter { void UpdateListOfGlobals() final; void UpdateListOfGlobalFunctions() final; void UpdateListOfTypes() final; - void SetClassInfo(TClass* cl, Bool_t reload = kFALSE, Bool_t silent = kFALSE) final; - - ECheckClassInfo CheckClassInfo(const char *name, Bool_t autoload, Bool_t isClassOrNamespaceOnly = kFALSE) final; + void SetClassInfo(TClass *cl, + Bool_t reload = kFALSE, + Bool_t silent = kFALSE, + ClassInfo_t *classInfo = nullptr) final; + + ECheckClassInfo CheckClassInfo(const char *name, + Bool_t autoload, + Bool_t isClassOrNamespaceOnly = kFALSE, + ClassInfo_t **classInfo = nullptr) final; Bool_t CheckClassTemplate(const char *name) final; Longptr_t Calc(const char* line, EErrorCode* error = nullptr) final; diff --git a/core/metacling/src/TClingClassInfo.cxx b/core/metacling/src/TClingClassInfo.cxx index 434996e2cb5aa..fd41b2c7c9eb3 100644 --- a/core/metacling/src/TClingClassInfo.cxx +++ b/core/metacling/src/TClingClassInfo.cxx @@ -121,12 +121,19 @@ TClingClassInfo::TClingClassInfo(cling::Interpreter *interp, Init(tag); } -TClingClassInfo::TClingClassInfo(cling::Interpreter *interp, - const Decl *D) - : TClingDeclInfo(nullptr), fInterp(interp), fFirstTime(true), fDescend(false), fIterAll(kTRUE), - fIsIter(false), fOffsetCache(0) +TClingClassInfo::TClingClassInfo(cling::Interpreter *interp, const Decl *D, const Type *T) + : TClingDeclInfo(nullptr), + fInterp(interp), + fFirstTime(true), + fDescend(false), + fIterAll(kTRUE), + fIsIter(false), + fOffsetCache(0) { Init(D); + // The type as found by the lookup, conserving typedefs like Double32_t + // (may be null). + fType = T; } void TClingClassInfo::AddBaseOffsetValue(const clang::Decl* decl, ptrdiff_t offset) diff --git a/core/metacling/src/TClingClassInfo.h b/core/metacling/src/TClingClassInfo.h index a4a9e96e0d0f3..d93340e9b9fef 100644 --- a/core/metacling/src/TClingClassInfo.h +++ b/core/metacling/src/TClingClassInfo.h @@ -96,7 +96,7 @@ class TClingClassInfo final : public TClingDeclInfo { explicit TClingClassInfo(cling::Interpreter *, Bool_t all = kTRUE); explicit TClingClassInfo(cling::Interpreter *, const char *classname, bool intantiateTemplate = kTRUE); explicit TClingClassInfo(cling::Interpreter *interp, const clang::Type &tag); - explicit TClingClassInfo(cling::Interpreter *interp, const clang::Decl *D); + explicit TClingClassInfo(cling::Interpreter *interp, const clang::Decl *D, const clang::Type *T = nullptr); TClingClassInfo &operator=(const TClingClassInfo &rhs) { // Copy all but the mutex