diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -120,6 +120,8 @@ /// \p File's parents. llvm::Optional getProjectInfo(PathRef File) const override; + bool blockUntilIdle(Deadline Timeout) const override; + private: Options Opts; @@ -152,6 +154,9 @@ }; llvm::Optional lookupCDB(CDBLookupRequest Request) const; + class BroadcastThread; + std::unique_ptr Broadcaster; + // Performs broadcast on governed files. void broadcastCDB(CDBLookupResult Res) const; diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -11,6 +11,7 @@ #include "SourceCode.h" #include "support/Logger.h" #include "support/Path.h" +#include "support/Threading.h" #include "support/ThreadsafeFS.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Tooling/ArgumentsAdjusters.h" @@ -22,12 +23,15 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringMap.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/FileUtilities.h" #include "llvm/Support/Path.h" #include "llvm/Support/Program.h" #include "llvm/Support/VirtualFileSystem.h" +#include #include +#include #include #include #include @@ -357,7 +361,7 @@ DirectoryBasedGlobalCompilationDatabase:: DirectoryBasedGlobalCompilationDatabase(const Options &Opts) - : Opts(Opts) { + : Opts(Opts), Broadcaster(std::make_unique(*this)) { if (Opts.CompileCommandsDir) OnlyDirCache = std::make_unique(*Opts.CompileCommandsDir); } @@ -472,25 +476,107 @@ Result.CDB = std::move(CDB); Result.PI.SourceRoot = DirCache->Path; - // FIXME: Maybe make the following part async, since this can block - // retrieval of compile commands. if (ShouldBroadcast) broadcastCDB(Result); return Result; } -void DirectoryBasedGlobalCompilationDatabase::broadcastCDB( - CDBLookupResult Result) const { - vlog("Broadcasting compilation database from {0}", Result.PI.SourceRoot); - assert(Result.CDB && "Trying to broadcast an invalid CDB!"); +// The broadcast thread announces files with new compile commands to the world. +// Primarily this is used to enqueue them for background indexing. +// +// It's on a separate thread because: +// - otherwise it would block the first parse of the initial file +// - we need to enumerate all files in the CDB, of which there are many +// - we (will) have to evaluate config for every file in the CDB, which is slow +class DirectoryBasedGlobalCompilationDatabase::BroadcastThread { + class Filter; + DirectoryBasedGlobalCompilationDatabase &Parent; + + std::mutex Mu; + std::condition_variable CV; + // Shutdown flag (CV is notified after writing). + // This is atomic so that broadcasts can also observe it and abort early. + std::atomic ShouldStop = {false}; + struct Task { + CDBLookupResult Lookup; + Context Ctx; + }; + std::deque Queue; + llvm::Optional ActiveTask; + std::thread Thread; // Must be last member. + + // Thread body: this is just the basic queue procesing boilerplate. + void run() { + std::unique_lock Lock(Mu); + while (true) { + bool Stopping = false; + CV.wait(Lock, [&] { + return (Stopping = ShouldStop.load(std::memory_order_acquire)) || + !Queue.empty(); + }); + if (Stopping) { + Queue.clear(); + CV.notify_all(); + return; + } + ActiveTask = std::move(Queue.front()); + Queue.pop_front(); - std::vector AllFiles = Result.CDB->getAllFiles(); + Lock.unlock(); + { + WithContext WithCtx(std::move(ActiveTask->Ctx)); + process(ActiveTask->Lookup); + } + Lock.lock(); + ActiveTask.reset(); + CV.notify_all(); + } + } + + // Inspects a new CDB and broadcasts the files it owns. + void process(const CDBLookupResult &T); + +public: + BroadcastThread(DirectoryBasedGlobalCompilationDatabase &Parent) + : Parent(Parent), Thread([this] { run(); }) {} + + void enqueue(CDBLookupResult Lookup) { + { + assert(!Lookup.PI.SourceRoot.empty()); + std::lock_guard Lock(Mu); + // New CDB takes precedence over any queued one for the same directory. + llvm::erase_if(Queue, [&](const Task &T) { + return T.Lookup.PI.SourceRoot == Lookup.PI.SourceRoot; + }); + Queue.push_back({std::move(Lookup), Context::current().clone()}); + } + CV.notify_all(); + } + + bool blockUntilIdle(Deadline Timeout) { + std::unique_lock Lock(Mu); + return wait(Lock, CV, Timeout, + [&] { return Queue.empty() && !ActiveTask.hasValue(); }); + } + + ~BroadcastThread() { + ShouldStop.store(true, std::memory_order_release); + CV.notify_all(); + Thread.join(); + } +}; + +void DirectoryBasedGlobalCompilationDatabase::BroadcastThread::process( + const CDBLookupResult &T) { + vlog("Broadcasting compilation database from {0}", T.PI.SourceRoot); + + std::vector AllFiles = T.CDB->getAllFiles(); // We assume CDB in CompileCommandsDir owns all of its entries, since we don't // perform any search in parent paths whenever it is set. - if (OnlyDirCache) { - assert(OnlyDirCache->Path == Result.PI.SourceRoot && + if (Parent.OnlyDirCache) { + assert(Parent.OnlyDirCache->Path == T.PI.SourceRoot && "Trying to broadcast a CDB outside of CompileCommandsDir!"); - OnCommandChanged.broadcast(std::move(AllFiles)); + Parent.OnCommandChanged.broadcast(std::move(AllFiles)); return; } @@ -505,18 +591,22 @@ return true; FileAncestors.push_back(It.first->getKey()); - return pathEqual(Path, Result.PI.SourceRoot); + return pathEqual(Path, T.PI.SourceRoot); }); } // Work out which ones have CDBs in them. // Given that we know that CDBs have been moved/generated, don't trust caches. // (This should be rare, so it's OK to add a little latency). constexpr auto IgnoreCache = std::chrono::steady_clock::time_point::max(); - auto DirectoryCaches = getDirectoryCaches(FileAncestors); + auto DirectoryCaches = Parent.getDirectoryCaches(FileAncestors); assert(DirectoryCaches.size() == FileAncestors.size()); for (unsigned I = 0; I < DirectoryCaches.size(); ++I) { bool ShouldBroadcast = false; - if (DirectoryCaches[I]->get(Opts.TFS, ShouldBroadcast, + if (ShouldStop.load(std::memory_order_acquire)) { + log("Giving up on broadcasting CDB, as we're shutting down"); + return; + } + if (DirectoryCaches[I]->get(Parent.Opts.TFS, ShouldBroadcast, /*FreshTime=*/IgnoreCache, /*FreshTimeMissing=*/IgnoreCache)) DirectoryHasCDB.find(FileAncestors[I])->setValue(true); @@ -528,7 +618,7 @@ // Independent of whether it has an entry for that file or not. actOnAllParentDirectories(File, [&](PathRef Path) { if (DirectoryHasCDB.lookup(Path)) { - if (pathEqual(Path, Result.PI.SourceRoot)) + if (pathEqual(Path, T.PI.SourceRoot)) // Make sure listeners always get a canonical path for the file. GovernedFiles.push_back(removeDots(File)); // Stop as soon as we hit a CDB. @@ -538,7 +628,18 @@ }); } - OnCommandChanged.broadcast(std::move(GovernedFiles)); + Parent.OnCommandChanged.broadcast(std::move(GovernedFiles)); +} + +void DirectoryBasedGlobalCompilationDatabase::broadcastCDB( + CDBLookupResult Result) const { + assert(Result.CDB && "Trying to broadcast an invalid CDB!"); + Broadcaster->enqueue(Result); +} + +bool DirectoryBasedGlobalCompilationDatabase::blockUntilIdle( + Deadline Timeout) const { + return Broadcaster->blockUntilIdle(Timeout); } llvm::Optional diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp --- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp +++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp @@ -217,11 +217,13 @@ }); DB.getCompileCommand(testPath("build/../a.cc")); + ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10))); EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(AllOf( EndsWith("a.cc"), Not(HasSubstr(".."))))); DiscoveredFiles.clear(); DB.getCompileCommand(testPath("build/gen.cc")); + ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10))); EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("gen.cc"))); } @@ -237,12 +239,14 @@ }); DB.getCompileCommand(testPath("a.cc")); + ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10))); EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("a.cc"), EndsWith("gen.cc"), EndsWith("gen2.cc"))); DiscoveredFiles.clear(); DB.getCompileCommand(testPath("build/gen.cc")); + ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10))); EXPECT_THAT(DiscoveredFiles, IsEmpty()); } }