[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 18 07:06:04 PST 2018

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`

  rCTE Clang Tools Extra



More information about the cfe-commits mailing list