kadircet marked an inline comment as done. kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/TUScheduler.cpp:661 + std::unique_lock<std::mutex> Lock(Mutex); + RequestsCV.wait(Lock, [this] { + // Block until we reiceve a preamble request, unless a preamble already ---------------- sammccall wrote: > Does LatestPreamble signal RequestsCV or just PreambleCV? > > Seems like it might be less error-prone to have just one CV, signalled when > preamble requests are scheduled, latest preamble becomes available, and on > shutdown. The spurious wakeups shouldn't be a real problem, right? > Does LatestPreamble signal RequestsCV or just PreambleCV? it only signals `PreambleCV`. But it makes no sense for this code path to block on it, as it is signalled by the same thread. So in theory we should see `LatestPreamble` and not start blocking in such case. > Seems like it might be less error-prone to have just one CV, signalled when > preamble requests are scheduled, latest preamble becomes available, and on > shutdown. The spurious wakeups shouldn't be a real problem, right? Yeah spurious wake-ups aren't really a problem. I thought having separate CVs sounded more clear, as predicates would still look the same even if we only had one CV. So it would enforce us to signal/wait on the right condition variable. Happy to have only a single one too, do you have any suggestions for its name? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80784/new/ https://reviews.llvm.org/D80784 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits