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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
