From 059ad4249e00389ce9e7b2002f8ff566f0454cc4 Mon Sep 17 00:00:00 2001
From: Vassil Vassilev <v.g.vassilev@gmail.com>
Date: Sat, 6 Mar 2021 16:05:21 +0000
Subject: [PATCH] [cxxmodules] Do not push a transaction per LoadModule.
Prior to this patch, we push/pop transaction for each LoadModule call. This is
reasonable if we assume modules are perfectly layered. That is, no eager
module deserialization require definitions from another module. This is hard to
achieve for dependent on ROOT codebases during their incremental migration
process.
The current patch push/pops once per loading of all modules. This should perform
slightly better and allow entangled modules to still load.
This patch should address the CMSSW Modules IB:
Assertion `OldBuilder->DeferredDeclsToEmit.empty() && "Should have emitted all decls deferred to emit."' failed.
0 0x00007ffff6f113d7 in raise () from /lib64/libc.so.6
1 0x00007ffff6f12ac8 in abort () from /lib64/libc.so.6
2 0x00007ffff6f0a1a6 in __assert_fail_base () from /lib64/libc.so.6
3 0x00007ffff6f0a252 in __assert_fail () from /lib64/libc.so.6
4 0x00007fffed24ed82 in clang::CodeGeneratorImpl::StartModule
5 0x00007fffed24d33e in clang::CodeGenerator::StartModule
6 0x00007fffed0664f1 in cling::IncrementalParser::StartModule
7 0x00007fffed066f84 in cling::IncrementalParser::codeGenTransaction
8 0x00007fffed066aec in cling::IncrementalParser::commitTransaction
9 0x00007fffecf1af2c in cling::Interpreter::PushTransactionRAII::pop
10 0x00007fffecf1ae72 in cling::Interpreter::PushTransactionRAII::~PushTransactionRAII
11 0x00007fffece78534 in ClingMemberIterInternal::DCIter::DCIter
12 0x00007fffece7712c in TClingMemberIter::TClingMemberIter
13 0x00007fffece7724c in TClingDataMemberIter::TClingDataMemberIter
14 0x00007fffece750b6 in TClingDataMemberInfo::TClingDataMemberInfo
15 0x00007fffecd25918 in TCling::DataMemberInfo_Factory
16 0x00007ffff6b0e33e in TListOfDataMembers::Load
17 0x00007ffff6ae539d in TClass::CreateListOfDataMembers
18 0x00007ffff6ae54d4 in TClass::GetListOfDataMembers
19 0x00007ffff6ae3dd1 in TClass::GetDataMember
20 0x00007ffff6b259a0 in ROOT::Detail::TSchemaRuleSet::AddRule
21 0x00007ffff6adee6c in TClass::AddRule (rule=0x572670 "HepMC::GenVertex m_event",
22 0x00007ffff6adead2 in (anonymous namespace)::ReadRulesContent
23 0x00007ffff6adec40 in TClass::ReadRules
24 0x00007fffecd0b3a0 in TCling::Initialize
There are several points to consider:
* We should understand why there are still DeferredDeclsToEmit;
* We should remove the `TClingMemberIter` from the init path as it is *very expensive*
---
core/metacling/src/TCling.cxx | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 27834209f55..0c9ac81b954 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -1049,7 +1049,6 @@ static bool LoadModule(const std::string &ModuleName, cling::Interpreter &interp
::Info("TCling::__LoadModule", "Preloading module %s. \n",
ModuleName.c_str());
- cling::Interpreter::PushTransactionRAII deserRAII(&interp);
return interp.loadModule(ModuleName, /*Complain=*/true);
}
@@ -1191,10 +1190,14 @@ static void RegisterCxxModules(cling::Interpreter &clingInterp)
{
if (!clingInterp.getCI()->getLangOpts().Modules)
return;
- // Setup core C++ modules if we have any to setup.
- // Load libc and stl first.
- // Load vcruntime module for windows
+ // Loading of a module might deserialize.
+ cling::Interpreter::PushTransactionRAII deserRAII(&clingInterp);
+
+ // Setup core C++ modules if we have any to setup.
+
+ // Load libc and stl first.
+ // Load vcruntime module for windows
#ifdef R__WIN32
LoadModule("vcruntime", clingInterp);
LoadModule("services", clingInterp);
@@ -2277,6 +2280,7 @@ void TCling::RegisterModule(const char* modulename,
// FIXME: We should only complain for modules which we know to exist. For example, we should not complain about
// modules such as GenVector32 because it needs to fall back to GenVector.
+ cling::Interpreter::PushTransactionRAII deserRAII(GetInterpreterImpl());
ModuleWasSuccessfullyLoaded = LoadModule(ModuleName, *fInterpreter);
if (!ModuleWasSuccessfullyLoaded) {
// Only report if we found the module in the modulemap.
--
GitLab