[clang-tools-extra] 27ade4b - Reland "[clangd] Always run preamble indexing on a separate thread"
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 19 07:32:07 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-07-19T16:30:46+02:00
New Revision: 27ade4b554774187d2c0afcf64cd16fa6d5f619d
URL: https://github.com/llvm/llvm-project/commit/27ade4b554774187d2c0afcf64cd16fa6d5f619d
DIFF: https://github.com/llvm/llvm-project/commit/27ade4b554774187d2c0afcf64cd16fa6d5f619d.diff
LOG: Reland "[clangd] Always run preamble indexing on a separate thread"
This reverts commit 92c0546941190973e9201a08fa10edf27cb6992d.
Prevents tsan issues by resetting ref-counted-pointers eagerly, before
passing the copies into a new thread.
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Preamble.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index af002e0cb2d689..29390196a6d977 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -68,11 +68,10 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
UpdateIndexCallbacks(FileIndex *FIndex,
ClangdServer::Callbacks *ServerCallbacks,
const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks,
- bool CollectInactiveRegions,
- const ClangdServer::Options &Opts)
+ bool CollectInactiveRegions)
: FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks),
- CollectInactiveRegions(CollectInactiveRegions), Opts(Opts) {}
+ CollectInactiveRegions(CollectInactiveRegions) {}
void onPreambleAST(
PathRef Path, llvm::StringRef Version, CapturedASTCtx ASTCtx,
@@ -94,7 +93,7 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
ASTCtx.getPreprocessor(), *PI);
};
- if (Opts.AsyncPreambleIndexing && Tasks) {
+ if (Tasks) {
Tasks->runAsync("Preamble indexing for:" + Path + Version,
std::move(Task));
} else
@@ -164,7 +163,6 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
std::shared_ptr<StdLibSet> Stdlib;
AsyncTaskRunner *Tasks;
bool CollectInactiveRegions;
- const ClangdServer::Options &Opts;
};
class DraftStoreFS : public ThreadsafeFS {
@@ -229,7 +227,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
std::make_unique<UpdateIndexCallbacks>(
DynamicIdx.get(), Callbacks, TFS,
IndexTasks ? &*IndexTasks : nullptr,
- PublishInactiveRegions, Opts));
+ PublishInactiveRegions));
// Adds an index to the stack, at higher priority than existing indexes.
auto AddIndex = [&](SymbolIndex *Idx) {
if (this->Index != nullptr) {
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 88b6d2f11d9a0b..2bc8f02ff38a4b 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -185,10 +185,6 @@ class ClangdServer {
/// 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.
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index f4547a5babf081..31b38d067b2727 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -11,6 +11,8 @@
#include "Compiler.h"
#include "Config.h"
#include "Diagnostics.h"
+#include "FS.h"
+#include "FeatureModule.h"
#include "Headers.h"
#include "Protocol.h"
#include "SourceCode.h"
@@ -20,8 +22,10 @@
#include "support/ThreadsafeFS.h"
#include "support/Trace.h"
#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Type.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticLex.h"
+#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
@@ -50,12 +54,17 @@
#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/raw_ostream.h"
+#include <cassert>
+#include <chrono>
#include <cstddef>
+#include <cstdint>
+#include <cstdlib>
#include <functional>
#include <memory>
#include <optional>
@@ -606,7 +615,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
});
llvm::IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
CompilerInstance::createDiagnostics(&CI.getDiagnosticOpts(),
- &PreambleDiagnostics, false);
+ &PreambleDiagnostics,
+ /*ShouldOwnClient=*/false);
const Config &Cfg = Config::current();
PreambleDiagnostics.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
@@ -653,8 +663,12 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
auto BuiltPreamble = PrecompiledPreamble::Build(
CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine,
Stats ? TimedFS : StatCacheFS, std::make_shared<PCHContainerOperations>(),
- StoreInMemory, /*StoragePath=*/StringRef(), CapturedInfo);
+ StoreInMemory, /*StoragePath=*/"", CapturedInfo);
PreambleTimer.stopTimer();
+ // Reset references to ref-counted-ptrs before executing the callbacks, to
+ // prevent resetting them concurrently.
+ PreambleDiagsEngine.reset();
+ CI.DiagnosticOpts.reset();
// When building the AST for the main file, we do want the function
// bodies.
More information about the cfe-commits
mailing list