kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
LGTM, thanks! ================ Comment at: clangd/index/Background.h:112 + const size_t BuildIndexPeriodMs; + std::atomic<bool> SymbolsUpdatedSinceLastIndex; + std::mutex IndexMu; ---------------- kadircet wrote: > kadircet wrote: > > 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` > well actually I believe we should use `wait_until` rathert than `wait_for`, > since we don't wanna stop waiting if we weren't stopped. As discussed offline, even with `wait_until` we still need the boolean flag to check if anything was updated. 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
