-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[cxxmodules] Preload all modules. #1350
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
Closed
Closed
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev
Previous commit
[cxxmodules] Preload all modules.
This does not seem to affect the startup performance of ROOT.
- Loading branch information
commit d6af9c5ec471203dd2f67797d1e4dc58db9db8a4
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1088,6 +1088,32 @@ inline bool TCling::TUniqueString::Append(const std::string& str) | |
| return notPresent; | ||
| } | ||
|
|
||
| static std::string ResolveModuleFileName(clang::Module *M, const clang::Preprocessor &PP) | ||
| { | ||
| const HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); | ||
|
|
||
| std::string ModuleFileName; | ||
| bool LoadFromPrebuiltModulePath = false; | ||
| // We try to load the module from the prebuilt module paths. If not | ||
| // successful, we then try to find it in the module cache. | ||
| if (!HSOpts.PrebuiltModulePaths.empty()) { | ||
| // Load the module from the prebuilt module path. | ||
| ModuleFileName = PP.getHeaderSearchInfo().getModuleFileName(M->Name, "", /*UsePrebuiltPath*/ true); | ||
| if (!ModuleFileName.empty()) | ||
| LoadFromPrebuiltModulePath = true; | ||
| } | ||
| if (!LoadFromPrebuiltModulePath && M) { | ||
| // Load the module from the module cache. | ||
| ModuleFileName = PP.getHeaderSearchInfo().getModuleFileName(M); | ||
| } | ||
| bool Exists = false; | ||
| { | ||
| std::ifstream f(ModuleFileName); | ||
| Exists = f.good(); | ||
| } | ||
| return (Exists) ? ModuleFileName : ""; | ||
| } | ||
|
|
||
| //////////////////////////////////////////////////////////////////////////////// | ||
| /// Loads the basic C++ modules that we require to run any ROOT program. | ||
| /// This is just supposed to make the declarations in their headers available | ||
|
|
@@ -1108,83 +1134,63 @@ static void LoadCoreModules(cling::Interpreter &interp) | |
| interp.declare("/*This is decl is to get a valid sloc...*/;", &T); | ||
| SourceLocation ValidLoc = T->decls_begin()->m_DGR.getSingleDecl()->getLocStart(); | ||
| clang::Sema &TheSema = CI.getSema(); | ||
| clang::Preprocessor &PP = CI.getPreprocessor(); | ||
| // createImplicitModuleImportForErrorRecovery creates decls. | ||
| cling::Interpreter::PushTransactionRAII RAII(&interp); | ||
| std::vector<std::pair<clang::Module*, std::string>> TopLevelModulesToLoad; | ||
| for (auto I = moduleMap.module_begin(), E = moduleMap.module_end(); I != E; ++I) { | ||
| clang::Module *M = I->second; | ||
| assert(M); | ||
| // We skip the modules in the modulemap which are not built. | ||
| // FIXME: We should iterate and build the not-yet-built-for-some-reason modules. See PR1147. | ||
| std::string ModuleFileName = ResolveModuleFileName(M, PP); | ||
| if (ModuleFileName == "") { | ||
| // if (M->IsSystem) | ||
| // Error("TCling::LoadCoreModules", "Cannot load system module %s because it was not generated", | ||
| // M->Name.c_str()); | ||
| // else | ||
| // Info("TCling::LoadCoreModules", "Cannot load module %s because it was not generated", M->Name.c_str()); | ||
| continue; | ||
| } | ||
| if (M->IsSystem && !M->IsMissingRequirement) { | ||
| CI.loadModuleFile(ModuleFileName.c_str()); | ||
| TheSema.createImplicitModuleImportForErrorRecovery(ValidLoc, M); | ||
| for (clang::Module *subModule : M->submodules()) | ||
| TheSema.createImplicitModuleImportForErrorRecovery(ValidLoc, subModule); | ||
| } | ||
| else if (!M->IsSystem && !M->IsMissingRequirement) | ||
| TopLevelModulesToLoad.push_back(std::make_pair(M, ModuleFileName)); | ||
| } | ||
|
|
||
| // List of core modules we need to load. | ||
| std::vector<std::string> neededCoreModuleNames = {"Core", "RIO"}; | ||
| std::vector<std::string> missingCoreModuleNames; | ||
|
|
||
| std::vector<clang::Module *> coreModules; | ||
|
|
||
| // Lookup the core modules in the modulemap by name. | ||
| for (std::string moduleName : neededCoreModuleNames) { | ||
| clang::Module *module = moduleMap.findModule(moduleName); | ||
| if (module) { | ||
| coreModules.push_back(module); | ||
| } else { | ||
| // If we can't find a needed module, we record that to report it later. | ||
| missingCoreModuleNames.push_back(moduleName); | ||
| } | ||
| } | ||
|
|
||
| // If we couldn't find some modules, so let's print an error message. | ||
| if (!missingCoreModuleNames.empty()) { | ||
| std::string MissingModuleNameList; | ||
| for (const std::string &name : missingCoreModuleNames) { | ||
| MissingModuleNameList += " " + name; | ||
| } | ||
|
|
||
| Error("TCling::LoadCoreModules", | ||
| "Internal error, couldn't find core " | ||
| "C++ modules in modulemap:%s\n", | ||
| MissingModuleNameList.c_str()); | ||
| } | ||
|
|
||
| // Collect all submodules in the found core modules. | ||
| for (size_t i = 0; i < coreModules.size(); ++i) { | ||
| for (clang::Module *subModule : coreModules[i]->submodules()) | ||
| coreModules.push_back(subModule); | ||
| } | ||
|
|
||
| // Now we collect all header files from the previously collected modules. | ||
| std::vector<StringRef> moduleHeaders; | ||
| for (clang::Module *module : coreModules) { | ||
| ROOT::TMetaUtils::foreachHeaderInModule( | ||
| *module, [&moduleHeaders](const clang::Module::Header &h) { moduleHeaders.push_back(h.NameAsWritten); }); | ||
| // Those two modules are required to be preloaded by ROOT. If they are not around we have a big problem. | ||
| // FIXME: We should probably tag Core and RIO as [system] modules in the module map. This would simplify this code. | ||
| clang::Module *CoreM = moduleMap.findModule("Core"); | ||
| clang::Module *RIOM = moduleMap.findModule("RIO"); | ||
| if (CoreM) { | ||
| CI.loadModuleFile(ResolveModuleFileName(CoreM, PP)); | ||
| TheSema.createImplicitModuleImportForErrorRecovery(ValidLoc, CoreM); | ||
| } | ||
|
|
||
| // Turn the list of found header files into C++ code that includes all of | ||
| // them. This will be wrapped into an `import` declaration by clang, so we | ||
| // only make those modules available, not actually textually include those | ||
| // headers. | ||
| // FIXME: Use clang::ASTReader::makeModuleVisible. | ||
| std::stringstream declarations; | ||
| for (StringRef H : moduleHeaders) { | ||
| declarations << "#include \"" << H.str() << "\"\n"; | ||
| else | ||
| Error("TCling::LoadCoreModules", "Internal error, couldn't find C++ module Core in modulemap."); | ||
| if (RIOM) { | ||
| CI.loadModuleFile(ResolveModuleFileName(RIOM, PP)); | ||
| TheSema.createImplicitModuleImportForErrorRecovery(ValidLoc, RIOM); | ||
| } | ||
| else | ||
| Error("TCling::LoadCoreModules", "Internal error, couldn't find C++ module RIO in modulemap."); | ||
|
|
||
| // C99 decided that it's a very good idea to name a macro `I` (the letter I). | ||
| // This seems to screw up nearly all the template code out there as `I` is | ||
| // common template parameter name and iterator variable name. | ||
| // Let's follow the GCC recommendation and undefine `I` in case any of the | ||
| // core modules have defined it: | ||
| // https://www.gnu.org/software/libc/manual/html_node/Complex-Numbers.html | ||
| declarations << "#ifdef I\n #undef I\n #endif\n"; | ||
|
|
||
| auto result = interp.declare(declarations.str()); | ||
|
|
||
| if (result != cling::Interpreter::CompilationResult::kSuccess) { | ||
| Error("TCling::LoadCoreModules", "Couldn't parse core headers: %s\n", declarations.str().c_str()); | ||
| // Loading all other modules in rootcling would mean injecting extra (needless) dependencies to the pcms. | ||
| bool fromRootCling = dlsym(RTLD_DEFAULT, "usedToIdentifyRootClingByDlSym"); | ||
| if (fromRootCling) | ||
| return; | ||
|
|
||
| // FIXME: The performance of loading all modules at start up is comparable to the non-modules one. However we can | ||
| // do better: use the global module index to avoid loading all modules and load only the ones that contain the | ||
| // looked up identifier. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we already asked Richard about that and he said it's only storing identifiers and therefor only useful in C (not C++). |
||
| for (auto M : TopLevelModulesToLoad) { | ||
| assert(M.first && M.second != ""); | ||
| // Skip Core and RIO: we alread loaded them. | ||
| if (M.first == CoreM || M.first == RIOM) | ||
| continue; | ||
| CI.loadModuleFile(M.second.c_str()); | ||
| TheSema.createImplicitModuleImportForErrorRecovery(ValidLoc, M.first); | ||
| } | ||
| } | ||
|
|
||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why not recursively loading the submodules here? Does it work? (never tested that).