[PATCH] D94606: [clangd] Move DirBasedCDB broadcasting onto its own thread.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:496
+  std::condition_variable CV;
+  std::atomic<bool> ShouldStop = {false}; // Must notify CV after writing.
+  std::deque<CDBLookupResult> Queue;
----------------
kadircet wrote:
> nit: we already hold the lock within the loop while reading. it is nice that we no longer need to acquire the lock during destructor, but is it worth the extra mental load that comes with memory orderings? (also IIRC, default load/store behaviors are already acquire-release)
Whoops, I split this patch in half and now this makes no sense.

The point is rather that we should interrupt the "evaluate the config for every file" loop (which will be part of process()) if ShouldStop is set. So we check this atomic inside that loop, without acquiring the lock.

(In practice maybe this is always "fast enough" that we should just run the whole loop, enqueue all the background indexing stuff, and then shut down afterwards?)

> (also IIRC, default load/store behaviors are already acquire-release)

Default is sequentially consistent, which is almost always more than we need.

---

I've added a check to the existing loop (which probes directory caches), less necessary but shows the idea. If you prefer to drop this entirely that's OK too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94606/new/

https://reviews.llvm.org/D94606