Skip to content

Commit de4ba70

Browse files
committed
[clangd] Move DirBasedCDB broadcasting onto its own thread.
This is on the critical path (it blocks getting the compile command for the first file). It's not trivially fast: it involves processing all filenames in the CDB and doing some IO to look for shadowing CDBs. And we may make this slower soon - making CDB configurable implies evaluating the config for each listed to see which ones really are owned by the broadcasted CDB. Differential Revision: https://reviews.llvm.org/D94606
1 parent 536a1b0 commit de4ba70

File tree

3 files changed

+126
-16
lines changed

3 files changed

+126
-16
lines changed

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

Lines changed: 117 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "SourceCode.h"
1212
#include "support/Logger.h"
1313
#include "support/Path.h"
14+
#include "support/Threading.h"
1415
#include "support/ThreadsafeFS.h"
1516
#include "clang/Frontend/CompilerInvocation.h"
1617
#include "clang/Tooling/ArgumentsAdjusters.h"
@@ -22,12 +23,15 @@
2223
#include "llvm/ADT/STLExtras.h"
2324
#include "llvm/ADT/ScopeExit.h"
2425
#include "llvm/ADT/SmallString.h"
26+
#include "llvm/ADT/StringMap.h"
2527
#include "llvm/Support/FileSystem.h"
2628
#include "llvm/Support/FileUtilities.h"
2729
#include "llvm/Support/Path.h"
2830
#include "llvm/Support/Program.h"
2931
#include "llvm/Support/VirtualFileSystem.h"
32+
#include <atomic>
3033
#include <chrono>
34+
#include <condition_variable>
3135
#include <string>
3236
#include <tuple>
3337
#include <vector>
@@ -357,7 +361,7 @@ bool DirectoryBasedGlobalCompilationDatabase::DirectoryCache::load(
357361

358362
DirectoryBasedGlobalCompilationDatabase::
359363
DirectoryBasedGlobalCompilationDatabase(const Options &Opts)
360-
: Opts(Opts) {
364+
: Opts(Opts), Broadcaster(std::make_unique<BroadcastThread>(*this)) {
361365
if (Opts.CompileCommandsDir)
362366
OnlyDirCache = std::make_unique<DirectoryCache>(*Opts.CompileCommandsDir);
363367
}
@@ -472,25 +476,107 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
472476
Result.CDB = std::move(CDB);
473477
Result.PI.SourceRoot = DirCache->Path;
474478

475-
// FIXME: Maybe make the following part async, since this can block
476-
// retrieval of compile commands.
477479
if (ShouldBroadcast)
478480
broadcastCDB(Result);
479481
return Result;
480482
}
481483

482-
void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
483-
CDBLookupResult Result) const {
484-
vlog("Broadcasting compilation database from {0}", Result.PI.SourceRoot);
485-
assert(Result.CDB && "Trying to broadcast an invalid CDB!");
484+
// The broadcast thread announces files with new compile commands to the world.
485+
// Primarily this is used to enqueue them for background indexing.
486+
//
487+
// It's on a separate thread because:
488+
// - otherwise it would block the first parse of the initial file
489+
// - we need to enumerate all files in the CDB, of which there are many
490+
// - we (will) have to evaluate config for every file in the CDB, which is slow
491+
class DirectoryBasedGlobalCompilationDatabase::BroadcastThread {
492+
class Filter;
493+
DirectoryBasedGlobalCompilationDatabase &Parent;
494+
495+
std::mutex Mu;
496+
std::condition_variable CV;
497+
// Shutdown flag (CV is notified after writing).
498+
// This is atomic so that broadcasts can also observe it and abort early.
499+
std::atomic<bool> ShouldStop = {false};
500+
struct Task {
501+
CDBLookupResult Lookup;
502+
Context Ctx;
503+
};
504+
std::deque<Task> Queue;
505+
llvm::Optional<Task> ActiveTask;
506+
std::thread Thread; // Must be last member.
507+
508+
// Thread body: this is just the basic queue procesing boilerplate.
509+
void run() {
510+
std::unique_lock<std::mutex> Lock(Mu);
511+
while (true) {
512+
bool Stopping = false;
513+
CV.wait(Lock, [&] {
514+
return (Stopping = ShouldStop.load(std::memory_order_acquire)) ||
515+
!Queue.empty();
516+
});
517+
if (Stopping) {
518+
Queue.clear();
519+
CV.notify_all();
520+
return;
521+
}
522+
ActiveTask = std::move(Queue.front());
523+
Queue.pop_front();
486524

487-
std::vector<std::string> AllFiles = Result.CDB->getAllFiles();
525+
Lock.unlock();
526+
{
527+
WithContext WithCtx(std::move(ActiveTask->Ctx));
528+
process(ActiveTask->Lookup);
529+
}
530+
Lock.lock();
531+
ActiveTask.reset();
532+
CV.notify_all();
533+
}
534+
}
535+
536+
// Inspects a new CDB and broadcasts the files it owns.
537+
void process(const CDBLookupResult &T);
538+
539+
public:
540+
BroadcastThread(DirectoryBasedGlobalCompilationDatabase &Parent)
541+
: Parent(Parent), Thread([this] { run(); }) {}
542+
543+
void enqueue(CDBLookupResult Lookup) {
544+
{
545+
assert(!Lookup.PI.SourceRoot.empty());
546+
std::lock_guard<std::mutex> Lock(Mu);
547+
// New CDB takes precedence over any queued one for the same directory.
548+
llvm::erase_if(Queue, [&](const Task &T) {
549+
return T.Lookup.PI.SourceRoot == Lookup.PI.SourceRoot;
550+
});
551+
Queue.push_back({std::move(Lookup), Context::current().clone()});
552+
}
553+
CV.notify_all();
554+
}
555+
556+
bool blockUntilIdle(Deadline Timeout) {
557+
std::unique_lock<std::mutex> Lock(Mu);
558+
return wait(Lock, CV, Timeout,
559+
[&] { return Queue.empty() && !ActiveTask.hasValue(); });
560+
}
561+
562+
~BroadcastThread() {
563+
ShouldStop.store(true, std::memory_order_release);
564+
CV.notify_all();
565+
Thread.join();
566+
}
567+
};
568+
569+
void DirectoryBasedGlobalCompilationDatabase::BroadcastThread::process(
570+
const CDBLookupResult &T) {
571+
vlog("Broadcasting compilation database from {0}", T.PI.SourceRoot);
572+
573+
std::vector<std::string> AllFiles = T.CDB->getAllFiles();
488574
// We assume CDB in CompileCommandsDir owns all of its entries, since we don't
489575
// perform any search in parent paths whenever it is set.
490-
if (OnlyDirCache) {
491-
assert(OnlyDirCache->Path == Result.PI.SourceRoot &&
576+
if (Parent.OnlyDirCache) {
577+
assert(Parent.OnlyDirCache->Path == T.PI.SourceRoot &&
492578
"Trying to broadcast a CDB outside of CompileCommandsDir!");
493-
OnCommandChanged.broadcast(std::move(AllFiles));
579+
Parent.OnCommandChanged.broadcast(std::move(AllFiles));
494580
return;
495581
}
496582

@@ -505,18 +591,22 @@ void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
505591
return true;
506592

507593
FileAncestors.push_back(It.first->getKey());
508-
return pathEqual(Path, Result.PI.SourceRoot);
594+
return pathEqual(Path, T.PI.SourceRoot);
509595
});
510596
}
511597
// Work out which ones have CDBs in them.
512598
// Given that we know that CDBs have been moved/generated, don't trust caches.
513599
// (This should be rare, so it's OK to add a little latency).
514600
constexpr auto IgnoreCache = std::chrono::steady_clock::time_point::max();
515-
auto DirectoryCaches = getDirectoryCaches(FileAncestors);
601+
auto DirectoryCaches = Parent.getDirectoryCaches(FileAncestors);
516602
assert(DirectoryCaches.size() == FileAncestors.size());
517603
for (unsigned I = 0; I < DirectoryCaches.size(); ++I) {
518604
bool ShouldBroadcast = false;
519-
if (DirectoryCaches[I]->get(Opts.TFS, ShouldBroadcast,
605+
if (ShouldStop.load(std::memory_order_acquire)) {
606+
log("Giving up on broadcasting CDB, as we're shutting down");
607+
return;
608+
}
609+
if (DirectoryCaches[I]->get(Parent.Opts.TFS, ShouldBroadcast,
520610
/*FreshTime=*/IgnoreCache,
521611
/*FreshTimeMissing=*/IgnoreCache))
522612
DirectoryHasCDB.find(FileAncestors[I])->setValue(true);
@@ -528,7 +618,7 @@ void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
528618
// Independent of whether it has an entry for that file or not.
529619
actOnAllParentDirectories(File, [&](PathRef Path) {
530620
if (DirectoryHasCDB.lookup(Path)) {
531-
if (pathEqual(Path, Result.PI.SourceRoot))
621+
if (pathEqual(Path, T.PI.SourceRoot))
532622
// Make sure listeners always get a canonical path for the file.
533623
GovernedFiles.push_back(removeDots(File));
534624
// Stop as soon as we hit a CDB.
@@ -538,7 +628,18 @@ void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
538628
});
539629
}
540630

541-
OnCommandChanged.broadcast(std::move(GovernedFiles));
631+
Parent.OnCommandChanged.broadcast(std::move(GovernedFiles));
632+
}
633+
634+
void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
635+
CDBLookupResult Result) const {
636+
assert(Result.CDB && "Trying to broadcast an invalid CDB!");
637+
Broadcaster->enqueue(Result);
638+
}
639+
640+
bool DirectoryBasedGlobalCompilationDatabase::blockUntilIdle(
641+
Deadline Timeout) const {
642+
return Broadcaster->blockUntilIdle(Timeout);
542643
}
543644

544645
llvm::Optional<ProjectInfo>

clang-tools-extra/clangd/GlobalCompilationDatabase.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ class DirectoryBasedGlobalCompilationDatabase
120120
/// \p File's parents.
121121
llvm::Optional<ProjectInfo> getProjectInfo(PathRef File) const override;
122122

123+
bool blockUntilIdle(Deadline Timeout) const override;
124+
123125
private:
124126
Options Opts;
125127

@@ -152,6 +154,9 @@ class DirectoryBasedGlobalCompilationDatabase
152154
};
153155
llvm::Optional<CDBLookupResult> lookupCDB(CDBLookupRequest Request) const;
154156

157+
class BroadcastThread;
158+
std::unique_ptr<BroadcastThread> Broadcaster;
159+
155160
// Performs broadcast on governed files.
156161
void broadcastCDB(CDBLookupResult Res) const;
157162

clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,13 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
217217
});
218218

219219
DB.getCompileCommand(testPath("build/../a.cc"));
220+
ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10)));
220221
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(AllOf(
221222
EndsWith("a.cc"), Not(HasSubstr("..")))));
222223
DiscoveredFiles.clear();
223224

224225
DB.getCompileCommand(testPath("build/gen.cc"));
226+
ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10)));
225227
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("gen.cc")));
226228
}
227229

@@ -237,12 +239,14 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
237239
});
238240

239241
DB.getCompileCommand(testPath("a.cc"));
242+
ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10)));
240243
EXPECT_THAT(DiscoveredFiles,
241244
UnorderedElementsAre(EndsWith("a.cc"), EndsWith("gen.cc"),
242245
EndsWith("gen2.cc")));
243246
DiscoveredFiles.clear();
244247

245248
DB.getCompileCommand(testPath("build/gen.cc"));
249+
ASSERT_TRUE(DB.blockUntilIdle(timeoutSeconds(10)));
246250
EXPECT_THAT(DiscoveredFiles, IsEmpty());
247251
}
248252
}

0 commit comments

Comments
 (0)