kadircet added inline comments.
================ Comment at: clangd/index/Background.cpp:352 + std::unique_lock<std::mutex> Lock(IndexMu); + if (ShouldStop) + break; ---------------- Is double checking really necessary? I suppose it is for the case that we miss the notification, if that's the case maybe put a comment? ================ Comment at: clangd/index/Background.h:112 + const size_t BuildIndexPeriodMs; + std::atomic<bool> SymbolsUpdatedSinceLastIndex; + std::mutex IndexMu; ---------------- ioeric wrote: > kadircet wrote: > > We already have a mutex and cv, maybe get rid of this one signal the CV > > whenever we have an update and sleep for `buildindexperiodms` before > > issuing the re-build? > `IndexCV` serves two purposes: 1) get notified when `ShouldStop` is set and > 2) timeout after `BuildIndexPeriodMs`. We wouldn't want to `sleep` here > because it can take too long to shutdown clangd if `BuildIndexPeriodMs` is > big. by `sleep` I still meant `IndexCV.wait_for` Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55770/new/ https://reviews.llvm.org/D55770 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
