[clang] Allocate Module instances in BumpPtrAllocator
#112795
Merged
Conversation
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
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Jan Svoboda (jansvoboda11) ChangesIn Full diff: https://github.com/llvm/llvm-project/pull/112795.diff 5 Files Affected:
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index e86f4303d732b8..9c5d33fbb562cc 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -48,6 +48,7 @@ namespace clang {
class FileManager;
class LangOptions;
+class ModuleMap;
class TargetInfo;
/// Describes the name of a module.
@@ -99,6 +100,15 @@ struct ASTFileSignature : std::array<uint8_t, 20> {
}
};
+/// Required to construct a Module.
+///
+/// This tag type is only constructible by ModuleMap, guaranteeing it ownership
+/// of all Module instances.
+class ModuleConstructorTag {
+ explicit ModuleConstructorTag() = default;
+ friend ModuleMap;
+};
+
/// Describes a module or submodule.
///
/// Aligned to 8 bytes to allow for llvm::PointerIntPair<Module *, 3>.
@@ -497,8 +507,9 @@ class alignas(8) Module {
std::vector<Conflict> Conflicts;
/// Construct a new module or submodule.
- Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
- bool IsFramework, bool IsExplicit, unsigned VisibilityID);
+ Module(ModuleConstructorTag, StringRef Name, SourceLocation DefinitionLoc,
+ Module *Parent, bool IsFramework, bool IsExplicit,
+ unsigned VisibilityID);
~Module();
@@ -749,7 +760,6 @@ class alignas(8) Module {
///
/// \returns The submodule if found, or NULL otherwise.
Module *findSubmodule(StringRef Name) const;
- Module *findOrInferSubmodule(StringRef Name);
/// Get the Global Module Fragment (sub-module) for this module, it there is
/// one.
diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index 2e28ff6823cb2a..75b567a347cb6c 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -93,9 +93,12 @@ class ModuleMap {
/// named LangOpts::CurrentModule, if we've loaded it).
Module *SourceModule = nullptr;
+ /// The allocator for all (sub)modules.
+ llvm::SpecificBumpPtrAllocator<Module> ModulesAlloc;
+
/// Submodules of the current module that have not yet been attached to it.
- /// (Ownership is transferred if/when we create an enclosing module.)
- llvm::SmallVector<std::unique_ptr<Module>, 8> PendingSubmodules;
+ /// (Relationship is set up if/when we create an enclosing module.)
+ llvm::SmallVector<Module *, 8> PendingSubmodules;
/// The top-level modules that are known.
llvm::StringMap<Module *> Modules;
@@ -502,6 +505,8 @@ class ModuleMap {
/// \returns The named module, if known; otherwise, returns null.
Module *findModule(StringRef Name) const;
+ Module *findOrInferSubmodule(Module *Parent, StringRef Name);
+
/// Retrieve a module with the given name using lexical name lookup,
/// starting at the given context.
///
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index fee372bce3a367..ad52fccff5dc7f 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -34,8 +34,9 @@
using namespace clang;
-Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
- bool IsFramework, bool IsExplicit, unsigned VisibilityID)
+Module::Module(ModuleConstructorTag, StringRef Name,
+ SourceLocation DefinitionLoc, Module *Parent, bool IsFramework,
+ bool IsExplicit, unsigned VisibilityID)
: Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent),
VisibilityID(VisibilityID), IsUnimportable(false),
HasIncompatibleModuleFile(false), IsAvailable(true),
@@ -58,11 +59,7 @@ Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
}
}
-Module::~Module() {
- for (auto *Submodule : SubModules) {
- delete Submodule;
- }
-}
+Module::~Module() = default;
static bool isPlatformEnvironment(const TargetInfo &Target, StringRef Feature) {
StringRef Platform = Target.getPlatformName();
@@ -361,21 +358,6 @@ Module *Module::findSubmodule(StringRef Name) const {
return SubModules[Pos->getValue()];
}
-Module *Module::findOrInferSubmodule(StringRef Name) {
- llvm::StringMap<unsigned>::const_iterator Pos = SubModuleIndex.find(Name);
- if (Pos != SubModuleIndex.end())
- return SubModules[Pos->getValue()];
- if (!InferSubmodules)
- return nullptr;
- Module *Result = new Module(Name, SourceLocation(), this, false, InferExplicitSubmodules, 0);
- Result->InferExplicitSubmodules = InferExplicitSubmodules;
- Result->InferSubmodules = InferSubmodules;
- Result->InferExportWildcard = InferExportWildcard;
- if (Result->InferExportWildcard)
- Result->Exports.push_back(Module::ExportDecl(nullptr, true));
- return Result;
-}
-
Module *Module::getGlobalModuleFragment() const {
assert(isNamedModuleUnit() && "We should only query the global module "
"fragment from the C++20 Named modules");
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 2aada51c71c503..fd6bc17ae9cdac 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -362,12 +362,7 @@ ModuleMap::ModuleMap(SourceManager &SourceMgr, DiagnosticsEngine &Diags,
MMapLangOpts.LineComment = true;
}
-ModuleMap::~ModuleMap() {
- for (auto &M : Modules)
- delete M.getValue();
- for (auto *M : ShadowModules)
- delete M;
-}
+ModuleMap::~ModuleMap() = default;
void ModuleMap::setTarget(const TargetInfo &Target) {
assert((!this->Target || this->Target == &Target) &&
@@ -831,6 +826,22 @@ Module *ModuleMap::findModule(StringRef Name) const {
return nullptr;
}
+Module *ModuleMap::findOrInferSubmodule(Module *Parent, StringRef Name) {
+ if (Module *SubM = Parent->findSubmodule(Name))
+ return SubM;
+ if (!Parent->InferSubmodules)
+ return nullptr;
+ Module *Result = new (ModulesAlloc.Allocate())
+ Module(ModuleConstructorTag{}, Name, SourceLocation(), Parent, false,
+ Parent->InferExplicitSubmodules, 0);
+ Result->InferExplicitSubmodules = Parent->InferExplicitSubmodules;
+ Result->InferSubmodules = Parent->InferSubmodules;
+ Result->InferExportWildcard = Parent->InferExportWildcard;
+ if (Result->InferExportWildcard)
+ Result->Exports.push_back(Module::ExportDecl(nullptr, true));
+ return Result;
+}
+
Module *ModuleMap::lookupModuleUnqualified(StringRef Name,
Module *Context) const {
for(; Context; Context = Context->Parent) {
@@ -857,8 +868,9 @@ std::pair<Module *, bool> ModuleMap::findOrCreateModule(StringRef Name,
return std::make_pair(Sub, false);
// Create a new module with this name.
- Module *Result = new Module(Name, SourceLocation(), Parent, IsFramework,
- IsExplicit, NumCreatedModules++);
+ Module *Result = new (ModulesAlloc.Allocate())
+ Module(ModuleConstructorTag{}, Name, SourceLocation(), Parent,
+ IsFramework, IsExplicit, NumCreatedModules++);
if (!Parent) {
if (LangOpts.CurrentModule == Name)
SourceModule = Result;
@@ -870,8 +882,9 @@ std::pair<Module *, bool> ModuleMap::findOrCreateModule(StringRef Name,
Module *ModuleMap::createGlobalModuleFragmentForModuleUnit(SourceLocation Loc,
Module *Parent) {
- auto *Result = new Module("<global>", Loc, Parent, /*IsFramework*/ false,
- /*IsExplicit*/ true, NumCreatedModules++);
+ auto *Result = new (ModulesAlloc.Allocate()) Module(
+ ModuleConstructorTag{}, "<global>", Loc, Parent, /*IsFramework=*/false,
+ /*IsExplicit=*/true, NumCreatedModules++);
Result->Kind = Module::ExplicitGlobalModuleFragment;
// If the created module isn't owned by a parent, send it to PendingSubmodules
// to wait for its parent.
@@ -888,9 +901,9 @@ ModuleMap::createImplicitGlobalModuleFragmentForModuleUnit(SourceLocation Loc,
// Note: Here the `IsExplicit` parameter refers to the semantics in clang
// modules. All the non-explicit submodules in clang modules will be exported
// too. Here we simplify the implementation by using the concept.
- auto *Result =
- new Module("<implicit global>", Loc, Parent, /*IsFramework=*/false,
- /*IsExplicit=*/false, NumCreatedModules++);
+ auto *Result = new (ModulesAlloc.Allocate())
+ Module(ModuleConstructorTag{}, "<implicit global>", Loc, Parent,
+ /*IsFramework=*/false, /*IsExplicit=*/false, NumCreatedModules++);
Result->Kind = Module::ImplicitGlobalModuleFragment;
return Result;
}
@@ -898,25 +911,23 @@ ModuleMap::createImplicitGlobalModuleFragmentForModuleUnit(SourceLocation Loc,
Module *
ModuleMap::createPrivateModuleFragmentForInterfaceUnit(Module *Parent,
SourceLocation Loc) {
- auto *Result =
- new Module("<private>", Loc, Parent, /*IsFramework*/ false,
- /*IsExplicit*/ true, NumCreatedModules++);
+ auto *Result = new (ModulesAlloc.Allocate()) Module(
+ ModuleConstructorTag{}, "<private>", Loc, Parent, /*IsFramework=*/false,
+ /*IsExplicit=*/true, NumCreatedModules++);
Result->Kind = Module::PrivateModuleFragment;
return Result;
}
Module *ModuleMap::createModuleUnitWithKind(SourceLocation Loc, StringRef Name,
Module::ModuleKind Kind) {
- auto *Result =
- new Module(Name, Loc, nullptr, /*IsFramework*/ false,
- /*IsExplicit*/ false, NumCreatedModules++);
+ auto *Result = new (ModulesAlloc.Allocate())
+ Module(ModuleConstructorTag{}, Name, Loc, nullptr, /*IsFramework=*/false,
+ /*IsExplicit=*/false, NumCreatedModules++);
Result->Kind = Kind;
// Reparent any current global module fragment as a submodule of this module.
- for (auto &Submodule : PendingSubmodules) {
+ for (auto &Submodule : PendingSubmodules)
Submodule->setParent(Result);
- Submodule.release(); // now owned by parent
- }
PendingSubmodules.clear();
return Result;
}
@@ -968,8 +979,9 @@ Module *ModuleMap::createHeaderUnit(SourceLocation Loc, StringRef Name,
assert(LangOpts.CurrentModule == Name && "module name mismatch");
assert(!Modules[Name] && "redefining existing module");
- auto *Result = new Module(Name, Loc, nullptr, /*IsFramework*/ false,
- /*IsExplicit*/ false, NumCreatedModules++);
+ auto *Result = new (ModulesAlloc.Allocate())
+ Module(ModuleConstructorTag{}, Name, Loc, nullptr, /*IsFramework=*/false,
+ /*IsExplicit=*/false, NumCreatedModules++);
Result->Kind = Module::ModuleHeaderUnit;
Modules[Name] = SourceModule = Result;
addHeader(Result, H, NormalHeader);
@@ -1082,9 +1094,9 @@ Module *ModuleMap::inferFrameworkModule(DirectoryEntryRef FrameworkDir,
if (!UmbrellaHeader)
return nullptr;
- Module *Result = new Module(ModuleName, SourceLocation(), Parent,
- /*IsFramework=*/true, /*IsExplicit=*/false,
- NumCreatedModules++);
+ Module *Result = new (ModulesAlloc.Allocate())
+ Module(ModuleConstructorTag{}, ModuleName, SourceLocation(), Parent,
+ /*IsFramework=*/true, /*IsExplicit=*/false, NumCreatedModules++);
InferredModuleAllowedBy[Result] = ModuleMapFID;
Result->IsInferred = true;
if (!Parent) {
@@ -1173,9 +1185,9 @@ Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
Module *ShadowingModule) {
// Create a new module with this name.
- Module *Result =
- new Module(Name, SourceLocation(), /*Parent=*/nullptr, IsFramework,
- /*IsExplicit=*/false, NumCreatedModules++);
+ Module *Result = new (ModulesAlloc.Allocate())
+ Module(ModuleConstructorTag{}, Name, SourceLocation(), /*Parent=*/nullptr,
+ IsFramework, /*IsExplicit=*/false, NumCreatedModules++);
Result->ShadowingModule = ShadowingModule;
Result->markUnavailable(/*Unimportable*/true);
ModuleScopeIDs[Result] = CurrentModuleScopeID;
diff --git a/clang/lib/Lex/Pragma.cpp b/clang/lib/Lex/Pragma.cpp
index 10f0ab7180e62d..6ec63b91df4bec 100644
--- a/clang/lib/Lex/Pragma.cpp
+++ b/clang/lib/Lex/Pragma.cpp
@@ -1752,6 +1752,7 @@ struct PragmaModuleBeginHandler : public PragmaHandler {
// Find the module we're entering. We require that a module map for it
// be loaded or implicitly loadable.
auto &HSI = PP.getHeaderSearchInfo();
+ auto &MM = HSI.getModuleMap();
Module *M = HSI.lookupModule(Current, ModuleName.front().second);
if (!M) {
PP.Diag(ModuleName.front().second,
@@ -1759,7 +1760,7 @@ struct PragmaModuleBeginHandler : public PragmaHandler {
return;
}
for (unsigned I = 1; I != ModuleName.size(); ++I) {
- auto *NewM = M->findOrInferSubmodule(ModuleName[I].first->getName());
+ auto *NewM = MM.findOrInferSubmodule(M, ModuleName[I].first->getName());
if (!NewM) {
PP.Diag(ModuleName[I].second, diag::err_pp_module_begin_no_submodule)
<< M->getFullModuleName() << ModuleName[I].first;
|
benlangmuir
approved these changes
Oct 21, 2024
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/881 Here is the relevant piece of the build log for the reference |
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Nov 4, 2024
In `clang-scan-deps`, we're creating lots of `Module` instances. Allocating them all in a bump-pointer allocator reduces the number of retired instructions by 1-1.5% on my workload. (cherry picked from commit 8bbd079)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
In
clang-scan-deps, we're creating lots ofModuleinstances. Allocating them all in a bump-pointer allocator reduces the number of retired instructions by 1-1.5% on my workload.