[core] Reduce symbol search only to when autoloading is enabled.

The llvm9 JIT issued callbacks when a symbol was missing and we reacted on it
by loading the relevant library. In root-project/root@9b2041e3 we have kept the
logic but now the JIT started querying more often even for symbols which are
okay to be missing. In turn that leads to scanning all libraries causing
performance issues.

This patch tries to limit this functionality only in contexts where automatic
loading is allowed. In addition when computing the offsets of a constant variable
declaration we compute the initializers instead of searching in the shared
objects.

core/metacling/src/TCling.cxx

@@ -4726,6 +4726,9 @@ TInterpreter::DeclId_t TCling::GetDataMember(ClassInfo_t *opaque_cl, const char
    DeclId_t d;
    TClingClassInfo *cl = (TClingClassInfo*)opaque_cl;
 
+   // Could trigger deserialization of decls.
+   cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl());
+
    if (cl) {
       d = cl->GetDataMember(name);
       // We check if the decl of the data member has an annotation which indicates
@@ -4756,8 +4759,6 @@ TInterpreter::DeclId_t TCling::GetDataMember(ClassInfo_t *opaque_cl, const char
    LookupResult R(SemaR, DName, SourceLocation(), Sema::LookupOrdinaryName,
                   Sema::ForExternalRedeclaration);
 
-   // Could trigger deserialization of decls.
-   cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl());
    cling::utils::Lookup::Named(&SemaR, R);
 
    LookupResult::Filter F = R.makeFilter();

core/metacling/src/TClingCallbacks.cxx

@@ -84,16 +84,22 @@ extern "C" {
 }
 
 class AutoloadLibraryMU : public llvm::orc::MaterializationUnit {
+   const TClingCallbacks &CB;
 public:
-   AutoloadLibraryMU(const std::string &Library, const llvm::orc::SymbolNameVector &Symbols)
-      : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), fLibrary(Library), fSymbols(Symbols)
+   AutoloadLibraryMU(const TClingCallbacks &cb, const std::string &Library, const llvm::orc::SymbolNameVector &Symbols)
+      : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), CB(cb), fLibrary(Library), fSymbols(Symbols)
    {
    }
 
    StringRef getName() const override { return "<Symbols from Autoloaded Library>"; }
 
    void materialize(std::unique_ptr<llvm::orc::MaterializationResponsibility> R) override
    {
+      if (!CB.IsAutoLoadingEnabled()) {
+         R->failMaterialization();
+         return;
+      }
+
       llvm::orc::SymbolMap loadedSymbols;
       llvm::orc::SymbolNameSet failedSymbols;
       bool loadedLibrary = false;
@@ -158,13 +164,18 @@ class AutoloadLibraryMU : public llvm::orc::MaterializationUnit {
 };
 
 class AutoloadLibraryGenerator : public llvm::orc::DefinitionGenerator {
+   const TClingCallbacks &CB;
 public:
-   AutoloadLibraryGenerator(cling::Interpreter *interp) : fInterpreter(interp) {}
+   AutoloadLibraryGenerator(cling::Interpreter *interp, const TClingCallbacks& cb)
+      : CB(cb), fInterpreter(interp) {}
 
    llvm::Error tryToGenerate(llvm::orc::LookupState &LS, llvm::orc::LookupKind K, llvm::orc::JITDylib &JD,
                              llvm::orc::JITDylibLookupFlags JDLookupFlags,
                              const llvm::orc::SymbolLookupSet &Symbols) override
    {
+      if (!CB.IsAutoLoadingEnabled())
+         llvm::Error::success();
+
       // If we get here, the symbols have not been found in the current process,
       // so no need to check that again. Instead search for the library that
       // provides the symbol and create one MaterializationUnit per library to
@@ -199,7 +210,7 @@ class AutoloadLibraryGenerator : public llvm::orc::DefinitionGenerator {
       }
 
       for (auto &&KV : found) {
-         auto MU = std::make_unique<AutoloadLibraryMU>(KV.first, std::move(KV.second));
+         auto MU = std::make_unique<AutoloadLibraryMU>(CB, KV.first, std::move(KV.second));
          if (auto Err = JD.define(MU))
             return Err;
       }
@@ -222,7 +233,7 @@ TClingCallbacks::TClingCallbacks(cling::Interpreter *interp, bool hasCodeGen) :
       m_Interpreter->declare("namespace __ROOT_SpecialObjects{}", &T);
       fROOTSpecialNamespace = dyn_cast<NamespaceDecl>(T->getFirstDecl().getSingleDecl());
 
-      interp->addGenerator(std::make_unique<AutoloadLibraryGenerator>(interp));
+      interp->addGenerator(std::make_unique<AutoloadLibraryGenerator>(interp, *this));
    }
 }
 

core/metacling/src/TClingCallbacks.h

@@ -58,7 +58,7 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
    void Initialize();
 
    void SetAutoLoadingEnabled(bool val = true) { fIsAutoLoading = val; }
-   bool IsAutoLoadingEnabled() { return fIsAutoLoading; }
+   bool IsAutoLoadingEnabled() const { return fIsAutoLoading; }
 
    void SetAutoParsingSuspended(bool val = true) { fIsAutoParsingSuspended = val; }
    bool IsAutoParsingSuspended() { return fIsAutoParsingSuspended; }

core/metacling/src/TClingDataMemberInfo.cxx

@@ -352,10 +352,9 @@ Longptr_t TClingDataMemberInfo::Offset()
       //   static constexpr Long64_t something = std::numeric_limits<Long64_t>::max();
       cling::Interpreter::PushTransactionRAII RAII(fInterp);
 
-      if (Longptr_t addr = reinterpret_cast<Longptr_t>(fInterp->getAddressOfGlobal(GlobalDecl(VD))))
-         return addr;
-      auto evalStmt = VD->ensureEvaluatedStmt();
-      if (evalStmt && evalStmt->Value) {
+      // We can't reassign constexpr or const variables. We can compute the
+      // initializer.
+      if (VD->hasInit() && (VD->isConstexpr() || VD->getType().isConstQualified())) {
          if (const APValue* val = VD->evaluateValue()) {
             if (VD->getType()->isIntegralType(C)) {
                return reinterpret_cast<Longptr_t>(val->getInt().getRawData());
@@ -388,6 +387,10 @@ Longptr_t TClingDataMemberInfo::Offset()
             } // not integral type
          } // have an APValue
       } // have an initializing value
+
+      // Try the slow operation.
+      if (Longptr_t addr = reinterpret_cast<Longptr_t>(fInterp->getAddressOfGlobal(GlobalDecl(VD))))
+         return addr;
    }
    // FIXME: We have to explicitly check for not enum constant because the
    // implementation of getAddressOfGlobal relies on mangling the name and in

core/metacling/test/TClingDataMemberInfoTests.cxx

@@ -220,3 +220,35 @@ class Outer {
    auto *dmInnerProtected = (TDataMember*)TClass::GetClass("DMLookup::Outer")->GetListOfDataMembers()->FindObject("InnerProtected");
    EXPECT_EQ(dmInnerProtected->Property(), kIsProtected | kIsClass);
 }
+
+TEST(TClingDataMemberInfo, Offset)
+{
+   gInterpreter->Declare("int ROOT_7459 = 42; ROOT_7459++;");
+   EXPECT_TRUE(43 == *(int*)gROOT->GetGlobal("ROOT_7459")->GetAddress());
+
+   gInterpreter->Declare("constexpr int var1 = 1;");
+   EXPECT_TRUE(1 == *(int*)gROOT->GetGlobal("var1")->GetAddress());
+
+   gInterpreter->Declare("static constexpr int var2 = -2;");
+   EXPECT_TRUE(-2 == *(int*)gROOT->GetGlobal("var2")->GetAddress());
+
+   gInterpreter->Declare("const float var3 = 3.1;");
+   EXPECT_FLOAT_EQ(3.1, *(float*)gROOT->GetGlobal("var3")->GetAddress());
+
+   gInterpreter->Declare("static const double var4 = 4.2;");
+   EXPECT_DOUBLE_EQ(4.2, *(double*)gROOT->GetGlobal("var4")->GetAddress());
+
+   // Make sure ROOT's Core constexpr constants work
+   EXPECT_EQ(3000, *(int*)gROOT->GetGlobal("kError")->GetAddress());
+
+#ifdef R__USE_CXXMODULES
+   // gGeoManager is defined in the Geom libraries and we want to make sure we
+   // do not load it when autoloading is off. We can only test this in modules
+   // mode because gGeoManager is not part of the PCH and non-modular ROOT has
+   // header parsing and autoloading coupled leading to redundant load of
+   // libGeom at gROOT->GetGlobal time.
+   TGlobal *GeoManagerInfo = gROOT->GetGlobal("gGeoManager");
+   TInterpreter::SuspendAutoLoadingRAII autoloadOff(gInterpreter);
+   EXPECT_TRUE(-1L == (ptrdiff_t)GeoManagerInfo->GetAddress());
+#endif // R__USE_CXXMODULES
+}

interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp

@@ -240,9 +240,20 @@ void IncrementalExecutor::replaceSymbol(const char* Name, void* Addr) const {
 
 void* IncrementalExecutor::getAddressOfGlobal(llvm::StringRef symbolName,
                                               bool* fromJIT /*=0*/) const {
-  constexpr bool includeHostSymbols = true;
 
-  void* address = m_JIT->getSymbolAddress(symbolName, includeHostSymbols);
+  // Try the fast path first, going via the dlsym and the dynamic linker.
+  if (void* Addr = const_cast<void*>(platform::DLSym(symbolName.str()))) {
+    if (fromJIT)
+      *fromJIT = false;
+    return Addr;
+  }
+
+  // Try iterating over the registered llvm libraries.
+  if (void* Addr = llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(symbolName.str())) {
+    if (fromJIT)
+      *fromJIT = false;
+    return Addr;
+  }
 
   // FIXME: If we split the loaded libraries into a separate JITDylib we should
   // be able to delete this code and use something like:
@@ -258,16 +269,14 @@ void* IncrementalExecutor::getAddressOfGlobal(llvm::StringRef symbolName,
   // }
   // fromJIT = false;
   // return nullptr;
-  if (fromJIT) {
-    // FIXME: See comments on DLSym below.
-    // We use dlsym to just tell if somethings was from the jit or not.
-#if !defined(_WIN32)
-    void* Addr = llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(symbolName.str());
-#else
-    void* Addr = const_cast<void*>(platform::DLSym(symbolName.str()));
-#endif
-    *fromJIT = !Addr;
-  }
+  constexpr bool includeHostSymbols = true;
+  void* address = m_JIT->getSymbolAddress(symbolName, includeHostSymbols);
+
+  // FIXME: Assuming that m_JIT->getSymbolAddress is techincally incorrect
+  // because an OrcV2 JIT can have a symbol generator that returns symbols via
+  // library scan.
+  if (fromJIT)
+    *fromJIT = true;
 
   return address;
 }