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