[clang] Reference-count ModuleCache non-intrusively
#164889
Open
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
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Jan Svoboda (jansvoboda11) ChangesThe Full diff: https://github.com/llvm/llvm-project/pull/164889.diff 14 Files Affected:
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 0177a1751bd60..524ec620c4076 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -265,7 +265,7 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath,
Preprocessor PP(PPOpts, *Diags, LangOpts, SourceMgr, HeaderInfo,
ModuleLoader);
- IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache();
+ std::shared_ptr<ModuleCache> ModCache = createCrossProcessModuleCache();
PCHContainerOperations PCHOperations;
CodeGenOptions CodeGenOpts;
ASTReader Reader(PP, *ModCache, /*ASTContext=*/nullptr,
diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h
index 3cea159afa33c..2889353473590 100644
--- a/clang/include/clang/Frontend/ASTUnit.h
+++ b/clang/include/clang/Frontend/ASTUnit.h
@@ -117,7 +117,7 @@ class ASTUnit {
IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics;
IntrusiveRefCntPtr<FileManager> FileMgr;
IntrusiveRefCntPtr<SourceManager> SourceMgr;
- IntrusiveRefCntPtr<ModuleCache> ModCache;
+ std::shared_ptr<ModuleCache> ModCache;
std::unique_ptr<HeaderSearch> HeaderInfo;
IntrusiveRefCntPtr<TargetInfo> Target;
std::shared_ptr<Preprocessor> PP;
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index 2403cbbb652dd..86ae9e8162b73 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -108,7 +108,7 @@ class CompilerInstance : public ModuleLoader {
IntrusiveRefCntPtr<SourceManager> SourceMgr;
/// The cache of PCM files.
- IntrusiveRefCntPtr<ModuleCache> ModCache;
+ std::shared_ptr<ModuleCache> ModCache;
/// Functor for getting the dependency preprocessor directives of a file.
std::unique_ptr<DependencyDirectivesGetter> GetDependencyDirectives;
@@ -201,7 +201,7 @@ class CompilerInstance : public ModuleLoader {
std::make_shared<CompilerInvocation>(),
std::shared_ptr<PCHContainerOperations> PCHContainerOps =
std::make_shared<PCHContainerOperations>(),
- ModuleCache *ModCache = nullptr);
+ std::shared_ptr<ModuleCache> ModCache = nullptr);
~CompilerInstance() override;
/// @name High-Level Operations
@@ -949,6 +949,7 @@ class CompilerInstance : public ModuleLoader {
void setExternalSemaSource(IntrusiveRefCntPtr<ExternalSemaSource> ESS);
ModuleCache &getModuleCache() const { return *ModCache; }
+ std::shared_ptr<ModuleCache> getModuleCachePtr() const { return ModCache; }
};
} // end namespace clang
diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h
index ec052c5c18e0a..c6795c5dc358a 100644
--- a/clang/include/clang/Serialization/ModuleCache.h
+++ b/clang/include/clang/Serialization/ModuleCache.h
@@ -10,7 +10,6 @@
#define LLVM_CLANG_SERIALIZATION_MODULECACHE_H
#include "clang/Basic/LLVM.h"
-#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include <ctime>
@@ -23,7 +22,7 @@ class InMemoryModuleCache;
/// The module cache used for compiling modules implicitly. This centralizes the
/// operations the compiler might want to perform on the cache.
-class ModuleCache : public RefCountedBase<ModuleCache> {
+class ModuleCache {
public:
/// May perform any work that only needs to be performed once for multiple
/// calls \c getLock() with the same module filename.
@@ -62,7 +61,7 @@ class ModuleCache : public RefCountedBase<ModuleCache> {
/// operated on by multiple processes. This instance must be used across all
/// \c CompilerInstance instances participating in building modules for single
/// translation unit in order to share the same \c InMemoryModuleCache.
-IntrusiveRefCntPtr<ModuleCache> createCrossProcessModuleCache();
+std::shared_ptr<ModuleCache> createCrossProcessModuleCache();
/// Shared implementation of `ModuleCache::maybePrune()`.
void maybePruneImpl(StringRef Path, time_t PruneInterval, time_t PruneAfter);
diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h
index 1eb74aee9787c..8ab70b6630f47 100644
--- a/clang/include/clang/Serialization/ModuleManager.h
+++ b/clang/include/clang/Serialization/ModuleManager.h
@@ -18,7 +18,6 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Serialization/ModuleFile.h"
#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
@@ -65,7 +64,7 @@ class ModuleManager {
FileManager &FileMgr;
/// Cache of PCM files.
- IntrusiveRefCntPtr<ModuleCache> ModCache;
+ ModuleCache &ModCache;
/// Knows how to unwrap module containers.
const PCHContainerReader &PCHContainerRdr;
@@ -306,7 +305,7 @@ class ModuleManager {
/// View the graphviz representation of the module graph.
void viewGraph();
- ModuleCache &getModuleCache() const { return *ModCache; }
+ ModuleCache &getModuleCache() const { return ModCache; }
};
} // namespace serialization
diff --git a/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h b/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h
index 213e60b39c199..81283f6aa5efc 100644
--- a/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h
+++ b/clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h
@@ -28,7 +28,7 @@ struct ModuleCacheEntries {
llvm::StringMap<std::unique_ptr<ModuleCacheEntry>> Map;
};
-IntrusiveRefCntPtr<ModuleCache>
+std::shared_ptr<ModuleCache>
makeInProcessModuleCache(ModuleCacheEntries &Entries);
} // namespace dependencies
} // namespace tooling
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 6cc7094846155..0ae72521d8cd6 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -1506,6 +1506,7 @@ void ASTUnit::transferASTDataFromCompilerInstance(CompilerInstance &CI) {
if (CI.hasTarget())
Target = CI.getTargetPtr();
Reader = CI.getASTReader();
+ ModCache = CI.getModuleCachePtr();
HadModuleLoaderFatalFailure = CI.hadModuleLoaderFatalFailure();
if (Invocation != CI.getInvocationPtr()) {
// This happens when Parse creates a copy of \c Invocation to modify.
@@ -2403,7 +2404,7 @@ bool ASTUnit::serialize(raw_ostream &OS) {
SmallString<128> Buffer;
llvm::BitstreamWriter Stream(Buffer);
- IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache();
+ std::shared_ptr<ModuleCache> ModCache = createCrossProcessModuleCache();
ASTWriter Writer(Stream, Buffer, *ModCache, *CodeGenOpts, {});
return serializeUnit(Writer, Buffer, getSema(), OS);
}
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index e3bf0eaa3c391..2482e3c649d6f 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -70,10 +70,11 @@ using namespace clang;
CompilerInstance::CompilerInstance(
std::shared_ptr<CompilerInvocation> Invocation,
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
- ModuleCache *ModCache)
- : ModuleLoader(/*BuildingModule=*/ModCache),
+ std::shared_ptr<ModuleCache> ModCache)
+ : ModuleLoader(/*BuildingModule=*/ModCache != nullptr),
Invocation(std::move(Invocation)),
- ModCache(ModCache ? ModCache : createCrossProcessModuleCache()),
+ ModCache(ModCache ? std::move(ModCache)
+ : createCrossProcessModuleCache()),
ThePCHContainerOperations(std::move(PCHContainerOps)) {
assert(this->Invocation && "Invocation must not be null");
}
@@ -1155,7 +1156,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
// CompilerInstance::CompilerInstance is responsible for finalizing the
// buffers to prevent use-after-frees.
auto InstancePtr = std::make_unique<CompilerInstance>(
- std::move(Invocation), getPCHContainerOperations(), &getModuleCache());
+ std::move(Invocation), getPCHContainerOperations(), ModCache);
auto &Instance = *InstancePtr;
auto &Inv = Instance.getInvocation();
diff --git a/clang/lib/Frontend/Rewrite/FrontendActions.cpp b/clang/lib/Frontend/Rewrite/FrontendActions.cpp
index f5656b3b190e9..ef6f9ccf87848 100644
--- a/clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ b/clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -244,7 +244,7 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener {
// Rewrite the contents of the module in a separate compiler instance.
CompilerInstance Instance(
std::make_shared<CompilerInvocation>(CI.getInvocation()),
- CI.getPCHContainerOperations(), &CI.getModuleCache());
+ CI.getPCHContainerOperations(), CI.getModuleCachePtr());
Instance.setVirtualFileSystem(CI.getVirtualFileSystemPtr());
Instance.createDiagnostics(
new ForwardingDiagnosticConsumer(CI.getDiagnosticClient()),
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index 9850956380423..aa6b3c0871ba4 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -148,6 +148,6 @@ class CrossProcessModuleCache : public ModuleCache {
};
} // namespace
-IntrusiveRefCntPtr<ModuleCache> clang::createCrossProcessModuleCache() {
- return llvm::makeIntrusiveRefCnt<CrossProcessModuleCache>();
+std::shared_ptr<ModuleCache> clang::createCrossProcessModuleCache() {
+ return std::make_shared<CrossProcessModuleCache>();
}
diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index 482c17649ed55..236fe20fdad7c 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -176,7 +176,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
if (NewModule->Kind == MK_ImplicitModule)
NewModule->InputFilesValidationTimestamp =
- ModCache->getModuleTimestamp(NewModule->FileName);
+ ModCache.getModuleTimestamp(NewModule->FileName);
// Load the contents of the module
if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {
@@ -330,7 +330,7 @@ void ModuleManager::moduleFileAccepted(ModuleFile *MF) {
ModuleManager::ModuleManager(FileManager &FileMgr, ModuleCache &ModCache,
const PCHContainerReader &PCHContainerRdr,
const HeaderSearch &HeaderSearchInfo)
- : FileMgr(FileMgr), ModCache(&ModCache), PCHContainerRdr(PCHContainerRdr),
+ : FileMgr(FileMgr), ModCache(ModCache), PCHContainerRdr(PCHContainerRdr),
HeaderSearchInfo(HeaderSearchInfo) {}
void ModuleManager::visit(llvm::function_ref<bool(ModuleFile &M)> Visitor,
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp
index 42f52d0ff6241..1fcbd3c6b38dc 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp
@@ -661,7 +661,7 @@ bool DependencyScanningAction::runInvocation(
// Create a compiler instance to handle the actual work.
auto ModCache = makeInProcessModuleCache(Service.getModuleCacheEntries());
ScanInstanceStorage.emplace(std::move(Invocation), std::move(PCHContainerOps),
- ModCache.get());
+ std::move(ModCache));
CompilerInstance &ScanInstance = *ScanInstanceStorage;
assert(!DiagConsumerFinished && "attempt to reuse finished consumer");
diff --git a/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp
index d1e543b438225..75dd2d8c21b9a 100644
--- a/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp
+++ b/clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp
@@ -114,7 +114,7 @@ class InProcessModuleCache : public ModuleCache {
};
} // namespace
-IntrusiveRefCntPtr<ModuleCache>
+std::shared_ptr<ModuleCache>
dependencies::makeInProcessModuleCache(ModuleCacheEntries &Entries) {
- return llvm::makeIntrusiveRefCnt<InProcessModuleCache>(Entries);
+ return std::make_shared<InProcessModuleCache>(Entries);
}
diff --git a/clang/unittests/Serialization/ModuleCacheTest.cpp b/clang/unittests/Serialization/ModuleCacheTest.cpp
index e9b8da3dba6af..00044ac729a3c 100644
--- a/clang/unittests/Serialization/ModuleCacheTest.cpp
+++ b/clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -145,7 +145,7 @@ TEST_F(ModuleCacheTest, CachedModuleNewPath) {
ASSERT_TRUE(Invocation2);
CompilerInstance Instance2(std::move(Invocation2),
Instance.getPCHContainerOperations(),
- &Instance.getModuleCache());
+ Instance.getModuleCachePtr());
Instance2.setVirtualFileSystem(CIOpts.VFS);
Instance2.setDiagnostics(Diags);
SyntaxOnlyAction Action2;
@@ -191,7 +191,7 @@ TEST_F(ModuleCacheTest, CachedModuleNewPathAllowErrors) {
ASSERT_TRUE(Invocation2);
CompilerInstance Instance2(std::move(Invocation2),
Instance.getPCHContainerOperations(),
- &Instance.getModuleCache());
+ Instance.getModuleCachePtr());
Instance2.setVirtualFileSystem(CIOpts.VFS);
Instance2.setDiagnostics(Diags);
SyntaxOnlyAction Action2;
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
The
ModuleCacheclass is currently reference-counted intrusively. As explained in #139584, this is problematic. This PR usesstd::shared_ptrto reference-countModuleCacheinstead, which clarifies what happens to its lifetime when constructingCompilerInstance, for example. This also makes the reference inModuleManagernon-owning, simplifying the ownership relationship further. TheASTUnit:: transferASTDataFromCompilerInstance()function now accounts for that by taking care to keep it alive.