We would like to move the preamble index out of the critical path.
This patch is an RFC to get feedback on the correct implementation and potential pitfalls to keep into consideration.
I am not entirely sure if the lazy AST initialisation would create using Preamble AST in parallel. I tried with tsan enabled clangd but it seems to work OK (at least for the cases I tried)
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D148088
UpdateIndexCallbacks(FileIndex *FIndex,
ClangdServer::Callbacks *ServerCallbacks,
const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks,
- bool CollectInactiveRegions)
- : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
- Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks),
- CollectInactiveRegions(CollectInactiveRegions) {}
-
- void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation &CI, ASTContext &Ctx,
- Preprocessor &PP,
- const CanonicalIncludes &CanonIncludes) override {
- // If this preamble uses a standard library we haven't seen yet, index it.
- if (FIndex)
- if (auto Loc = Stdlib->add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
- indexStdlib(CI, std::move(*Loc));
+ bool CollectInactiveRegions,
+ const ClangdServer::Options &Opts)
+ : FIndex(FIndex), ServerCallbacks(ServerCallbacks),
+ TFS(TFS), Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks),
+ CollectInactiveRegions(CollectInactiveRegions), Opts(Opts) {}
+
+ void onPreambleAST(
+ PathRef Path, llvm::StringRef Version, CapturedASTCtx ASTCtx,
+ const std::shared_ptr<const CanonicalIncludes> CanonIncludes) override {
+
+ if (!FIndex)
+ return;
+
+ auto &PP = ASTCtx.getPreprocessor();
+ auto &CI = ASTCtx.getCompilerInvocation();
+ if (auto Loc = Stdlib->add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
+ indexStdlib(CI, std::move(*Loc));
+
+ // FIndex outlives the UpdateIndexCallbacks.
+ auto Task = [FIndex(FIndex), Path(Path.str()), Version(Version.str()),
+ ASTCtx(std::move(ASTCtx)),
+ CanonIncludes(CanonIncludes)]() mutable {
+ trace::Span Tracer("PreambleIndexing");
+ FIndex->updatePreamble(Path, Version, ASTCtx.getASTContext(),
+ ASTCtx.getPreprocessor(), *CanonIncludes);
+ };
- if (FIndex)
- FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes);
+ if (Opts.AsyncPreambleIndexing && Tasks) {
+ Tasks->runAsync("Preamble indexing for:" + Path + Version,
+ std::move(Task));
+ } else
+ Task();
}
void indexStdlib(const CompilerInvocation &CI, StdLibLocation Loc) {
std::shared_ptr<StdLibSet> Stdlib;
AsyncTaskRunner *Tasks;
bool CollectInactiveRegions;
+ const ClangdServer::Options &Opts;
};
class DraftStoreFS : public ThreadsafeFS {
std::make_unique<UpdateIndexCallbacks>(
DynamicIdx.get(), Callbacks, TFS,
IndexTasks ? &*IndexTasks : nullptr,
- PublishInactiveRegions));
+ PublishInactiveRegions, Opts));
// Adds an index to the stack, at higher priority than existing indexes.
auto AddIndex = [&](SymbolIndex *Idx) {
if (this->Index != nullptr) {
/// regions in the document.
bool PublishInactiveRegions = false;
+ /// Whether to run preamble indexing asynchronously in an independent
+ /// thread.
+ bool AsyncPreambleIndexing = false;
+
explicit operator TUScheduler::Options() const;
};
// Sensible default options for use in tests.
// non-preamble includes below.
CanonicalIncludes CanonIncludes;
if (Preamble)
- CanonIncludes = Preamble->CanonIncludes;
+ CanonIncludes = *Preamble->CanonIncludes;
else
CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts());
std::unique_ptr<CommentHandler> IWYUHandler =
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/TokenKinds.h"
+#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Serialization/ASTReader.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
class CppFilePreambleCallbacks : public PreambleCallbacks {
public:
CppFilePreambleCallbacks(
- PathRef File, PreambleParsedCallback ParsedCallback,
- PreambleBuildStats *Stats, bool ParseForwardingFunctions,
+ PathRef File, PreambleBuildStats *Stats, bool ParseForwardingFunctions,
std::function<void(CompilerInstance &)> BeforeExecuteCallback)
- : File(File), ParsedCallback(ParsedCallback), Stats(Stats),
+ : File(File), Stats(Stats),
ParseForwardingFunctions(ParseForwardingFunctions),
BeforeExecuteCallback(std::move(BeforeExecuteCallback)) {}
}
CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); }
+ std::optional<CapturedASTCtx> takeLife() { return std::move(CapturedCtx); }
+
bool isMainFileIncludeGuarded() const { return IsMainFileIncludeGuarded; }
void AfterExecute(CompilerInstance &CI) override {
- if (ParsedCallback) {
- trace::Span Tracer("Running PreambleCallback");
- ParsedCallback(CI.getASTContext(), CI.getPreprocessor(), CanonIncludes);
+ // As part of the Preamble compilation, ASTConsumer
+ // PrecompilePreambleConsumer/PCHGenerator is setup. This would be called
+ // when Preamble consists of modules. Therefore while capturing AST context,
+ // we have to reset ast consumer and ASTMutationListener.
+ if (CI.getASTReader()) {
+ CI.getASTReader()->setDeserializationListener(nullptr);
+ // This just sets consumer to null when DeserializationListener is null.
+ CI.getASTReader()->StartTranslationUnit(nullptr);
}
+ CI.getASTContext().setASTMutationListener(nullptr);
+ CapturedCtx.emplace(CI);
const SourceManager &SM = CI.getSourceManager();
const FileEntry *MainFE = SM.getFileEntryForID(SM.getMainFileID());
private:
PathRef File;
- PreambleParsedCallback ParsedCallback;
IncludeStructure Includes;
CanonicalIncludes CanonIncludes;
include_cleaner::PragmaIncludes Pragmas;
PreambleBuildStats *Stats;
bool ParseForwardingFunctions;
std::function<void(CompilerInstance &)> BeforeExecuteCallback;
+ std::optional<CapturedASTCtx> CapturedCtx;
};
// Represents directives other than includes, where basic textual information is
CI.getPreprocessorOpts().WriteCommentListToPCH = false;
CppFilePreambleCallbacks CapturedInfo(
- FileName, PreambleCallback, Stats,
- Inputs.Opts.PreambleParseForwardingFunctions,
+ FileName, Stats, Inputs.Opts.PreambleParseForwardingFunctions,
[&ASTListeners](CompilerInstance &CI) {
for (const auto &L : ASTListeners)
L->beforeExecute(CI);
auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
llvm::SmallString<32> AbsFileName(FileName);
VFS->makeAbsolute(AbsFileName);
- auto StatCache = std::make_
unique<PreambleFileStatusCache>(AbsFileName);
+ auto StatCache = std::make_
shared<PreambleFileStatusCache>(AbsFileName);
auto StatCacheFS = StatCache->getProducingFS(VFS);
llvm::IntrusiveRefCntPtr<TimerFS> TimedFS(new TimerFS(StatCacheFS));
Result->Pragmas = CapturedInfo.takePragmaIncludes();
Result->Macros = CapturedInfo.takeMacros();
Result->Marks = CapturedInfo.takeMarks();
- Result->CanonIncludes = CapturedInfo.takeCanonicalIncludes();
- Result->StatCache = std::move(StatCache);
+ Result->CanonIncludes = std::make_shared<const CanonicalIncludes>(
+ (CapturedInfo.takeCanonicalIncludes()));
+ Result->StatCache = StatCache;
Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded();
+ if (PreambleCallback) {
+ trace::Span Tracer("Running PreambleCallback");
+ auto Ctx = CapturedInfo.takeLife();
+ // Stat cache is thread safe only when there are no producers. Hence
+ // change the VFS underneath to a consuming fs.
+ Ctx->getFileManager().setVirtualFileSystem(
+ Result->StatCache->getConsumingFS(VFS));
+ // While extending the life of FileMgr and VFS, StatCache should also be
+ // extended.
+ Ctx->setStatCache(Result->StatCache);
+ PreambleCallback(std::move(*Ctx), Result->CanonIncludes);
+ }
return Result;
}
namespace clang {
namespace clangd {
+/// The captured AST conext.
+/// Keeps necessary structs for an ASTContext and Preprocessor alive.
+/// This enables consuming them after context that produced the AST is gone.
+/// (e.g. indexing a preamble ast on a separate thread). ASTContext stored
+/// inside is still not thread-safe.
+
+struct CapturedASTCtx {
+public:
+ CapturedASTCtx(CompilerInstance &Clang)
+ : Invocation(Clang.getInvocationPtr()),
+ Diagnostics(Clang.getDiagnosticsPtr()), Target(Clang.getTargetPtr()),
+ AuxTarget(Clang.getAuxTarget()), FileMgr(Clang.getFileManagerPtr()),
+ SourceMgr(Clang.getSourceManagerPtr()), PP(Clang.getPreprocessorPtr()),
+ Context(Clang.getASTContextPtr()) {}
+
+ CapturedASTCtx(const CapturedASTCtx &) = delete;
+ CapturedASTCtx &operator=(const CapturedASTCtx &) = delete;
+ CapturedASTCtx(CapturedASTCtx &&) = default;
+ CapturedASTCtx &operator=(CapturedASTCtx &&) = default;
+
+ ASTContext &getASTContext() { return *Context; }
+ Preprocessor &getPreprocessor() { return *PP; }
+ CompilerInvocation &getCompilerInvocation() { return *Invocation; }
+ FileManager &getFileManager() { return *FileMgr; }
+ void setStatCache(std::shared_ptr<PreambleFileStatusCache> StatCache) {
+ this->StatCache = StatCache;
+ }
+
+private:
+ std::shared_ptr<CompilerInvocation> Invocation;
+ IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics;
+ IntrusiveRefCntPtr<TargetInfo> Target;
+ IntrusiveRefCntPtr<TargetInfo> AuxTarget;
+ IntrusiveRefCntPtr<FileManager> FileMgr;
+ IntrusiveRefCntPtr<SourceManager> SourceMgr;
+ std::shared_ptr<Preprocessor> PP;
+ IntrusiveRefCntPtr<ASTContext> Context;
+ std::shared_ptr<PreambleFileStatusCache> StatCache;
+};
+
/// The parsed preamble and associated data.
///
/// As we must avoid re-parsing the preamble, any information that can only
std::vector<PragmaMark> Marks;
// Cache of FS operations performed when building the preamble.
// When reusing a preamble, this cache can be consumed to save IO.
- std::
unique_ptr<PreambleFileStatusCache> StatCache;
- CanonicalIncludes CanonIncludes;
+ std::
shared_ptr<PreambleFileStatusCache> StatCache;
+ std::shared_ptr<const CanonicalIncludes> CanonIncludes;
// Whether there was a (possibly-incomplete) include-guard on the main file.
// We need to propagate this information "by hand" to subsequent parses.
bool MainIsIncludeGuarded = false;
};
-using PreambleParsedCallback = std::function<void(ASTContext &, Preprocessor &,
- const CanonicalIncludes &)>;
+using PreambleParsedCallback =
+ std::function<void(CapturedASTCtx ASTCtx,
+ std::shared_ptr<const CanonicalIncludes> CanonIncludes)>;
/// Timings and statistics from the premble build. Unlike PreambleData, these
/// do not need to be stored for later, but can be useful for logging, metrics,
bool IsFirstPreamble = !LatestBuild;
LatestBuild = clang::clangd::buildPreamble(
FileName, *Req.CI, Inputs, StoreInMemory,
- [&](ASTContext &Ctx, Preprocessor &PP,
- const CanonicalIncludes &CanonIncludes) {
- Callbacks.onPreambleAST(FileName, Inputs.Version, *Req.CI, Ctx, PP,
+ [&](CapturedASTCtx ASTCtx,
+ std::shared_ptr<const CanonicalIncludes> CanonIncludes) {
+ Callbacks.onPreambleAST(FileName, Inputs.Version, std::move(ASTCtx),
CanonIncludes);
},
&Stats);
/// contains only AST nodes from the #include directives at the start of the
/// file. AST node in the current file should be observed on onMainAST call.
virtual void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation &CI, ASTContext &Ctx,
- Preprocessor &PP, const CanonicalIncludes &) {}
+ CapturedASTCtx Ctx,
+ const std::shared_ptr<const CanonicalIncludes>) {}
/// The argument function is run under the critical section guarding against
/// races when closing the files.
// Build preamble and AST, and index them.
bool buildAST() {
log("Building preamble...");
- Preamble = buildPreamble(File, *Invocation, Inputs, /*StoreInMemory=*/true,
- [&](ASTContext &Ctx, Preprocessor &PP,
- const CanonicalIncludes &Includes) {
- if (!Opts.BuildDynamicSymbolIndex)
- return;
- log("Indexing headers...");
- Index.updatePreamble(File, /*Version=*/"null",
- Ctx, PP, Includes);
- });
+ Preamble = buildPreamble(
+ File, *Invocation, Inputs, /*StoreInMemory=*/true,
+ [&](CapturedASTCtx Ctx,
+ const std::shared_ptr<const CanonicalIncludes> Includes) {
+ if (!Opts.BuildDynamicSymbolIndex)
+ return;
+ log("Indexing headers...");
+ Index.updatePreamble(File, /*Version=*/"null", Ctx.getASTContext(),
+ Ctx.getPreprocessor(), *Includes);
+ });
if (!Preamble) {
elog("Failed to build preamble");
return false;
FileIndex Index;
bool IndexUpdated = false;
- buildPreamble(FooCpp, *CI, PI,
- /*StoreInMemory=*/true,
- [&](ASTContext &Ctx, Preprocessor &PP,
- const CanonicalIncludes &CanonIncludes) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.updatePreamble(FooCpp, /*Version=*/"null", Ctx, PP,
- CanonIncludes);
- });
+ buildPreamble(
+ FooCpp, *CI, PI,
+ /*StoreInMemory=*/true,
+ [&](CapturedASTCtx ASTCtx,
+ const std::shared_ptr<const CanonicalIncludes> CanonIncludes) {
+ auto &Ctx = ASTCtx.getASTContext();
+ auto &PP = ASTCtx.getPreprocessor();
+ EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
+ IndexUpdated = true;
+ Index.updatePreamble(FooCpp, /*Version=*/"null", Ctx, PP,
+ *CanonIncludes);
+ });
ASSERT_TRUE(IndexUpdated);
// Check the index contains symbols from the preamble, but not from the main
public:
BlockPreambleThread(llvm::StringRef BlockVersion, Notification &N)
: BlockVersion(BlockVersion), N(N) {}
- void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation &, ASTContext &Ctx,
- Preprocessor &, const CanonicalIncludes &) override {
+ void
+ onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx,
+ const std::shared_ptr<const CanonicalIncludes>) override {
if (Version == BlockVersion)
N.wait();
}
BlockPreambleThread(Notification &UnblockPreamble, DiagsCB CB)
: UnblockPreamble(UnblockPreamble), CB(std::move(CB)) {}
- void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation &, ASTContext &Ctx,
- Preprocessor &, const CanonicalIncludes &) override {
+ void
+ onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx,
+ const std::shared_ptr<const CanonicalIncludes>) override {
if (BuildBefore)
ASSERT_TRUE(UnblockPreamble.wait(timeoutSeconds(5)))
<< "Expected notification";
std::vector<std::string> &Filenames;
CaptureBuiltFilenames(std::vector<std::string> &Filenames)
: Filenames(Filenames) {}
- void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation &CI, ASTContext &Ctx,
- Preprocessor &PP, const CanonicalIncludes &) override {
+ void
+ onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx,
+ const std::shared_ptr<const CanonicalIncludes>) override {
// Deliberately no synchronization.
// The PreambleThrottler should serialize these calls, if not then tsan
// will find a bug here.
continue;
TU.Code = Input.second.Code;
TU.Filename = Input.first().str();
- TU.preamble([&](ASTContext &Ctx, Preprocessor &PP,
- const CanonicalIncludes &CanonIncludes) {
- Index->updatePreamble(testPath(Input.first()), "null", Ctx, PP,
- CanonIncludes);
- });
+ TU.preamble(
+ [&](CapturedASTCtx ASTCtx,
+ const std::shared_ptr<const CanonicalIncludes> CanonIncludes) {
+ auto &Ctx = ASTCtx.getASTContext();
+ auto &PP = ASTCtx.getPreprocessor();
+ Index->updatePreamble(testPath(Input.first()), "null", Ctx, PP,
+ *CanonIncludes);
+ });
ParsedAST MainAST = TU.build();
Index->updateMain(testPath(Input.first()), MainAST);
}
#include "clang/AST/ASTConsumer.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Frontend/Utils.h"
return *Invocation;
}
+ std::shared_ptr<CompilerInvocation> getInvocationPtr() { return Invocation; }
+
/// setInvocation - Replace the current invocation.
void setInvocation(std::shared_ptr<CompilerInvocation> Value);
return *Diagnostics;
}
+ IntrusiveRefCntPtr<DiagnosticsEngine> getDiagnosticsPtr() const {
+ assert(Diagnostics && "Compiler instance has no diagnostics!");
+ return Diagnostics;
+ }
+
/// setDiagnostics - Replace the current diagnostics engine.
void setDiagnostics(DiagnosticsEngine *Value);
return *Target;
}
+ IntrusiveRefCntPtr<TargetInfo> getTargetPtr() const {
+ assert(Target && "Compiler instance has no target!");
+ return Target;
+ }
+
/// Replace the current Target.
void setTarget(TargetInfo *Value);
return *FileMgr;
}
+ IntrusiveRefCntPtr<FileManager> getFileManagerPtr() const {
+ assert(FileMgr && "Compiler instance has no file manager!");
+ return FileMgr;
+ }
+
void resetAndLeakFileManager() {
llvm::BuryPointer(FileMgr.get());
FileMgr.resetWithoutRelease();
return *SourceMgr;
}
+ IntrusiveRefCntPtr<SourceManager> getSourceManagerPtr() const {
+ assert(SourceMgr && "Compiler instance has no source manager!");
+ return SourceMgr;
+ }
+
void resetAndLeakSourceManager() {
llvm::BuryPointer(SourceMgr.get());
SourceMgr.resetWithoutRelease();
return *Context;
}
+ IntrusiveRefCntPtr<ASTContext> getASTContextPtr() const {
+ assert(Context && "Compiler instance has no AST context!");
+ return Context;
+ }
+
void resetAndLeakASTContext() {
llvm::BuryPointer(Context.get());
Context.resetWithoutRelease();
projects / platform / upstream / llvm.git / commitdiff