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

Tue Dec 18 02:10:06 PST 2018

ioeric marked an inline comment as done.
ioeric added a comment.

In D55770#1333271 <https://reviews.llvm.org/D55770#1333271>, @dblaikie wrote:

> (from the peanut gallery): Usually what I'd expect here is "reindexing will occur X milliseconds after a write, covering anything written up until that point" - so it doesn't reindex unnecessarily if the files are unmodified/sitting idle, and it doesn't thrash reindexing for multiple changes in rapid succession (such as hitting the "save all" button in some text editor - or having made a few other quick changes one after another maybe), but still provides reasonable up-to-date data for the user.
>
> (but I don't know anything about clangd & maybe what I'm saying makes no sense/isn't relevant)

Thanks for the input David.

The term `indexing` in clangd is getting bit overloaded :( There are two kinds of "indexing " happening in the background index: 1) parse and collect symbols from the TU and 2) build a symbol index (e.g. for fast fuzzy search. this is the real index!) for the symbols. (1) happens in a manner similar to what you described (e.g. we don't reindex files with the same hash). This patch addresses (2). As there can be multiple threads collecting symbols concurrently, it's not ideal to rebuild the symbol index (data structure) for each index action; thus, we are making the index rebuild only run periodically.

Happen to chat more and provide more details if you are interested.

================
Comment at: clangd/index/Background.cpp:464
+  if (BuildIndexPeriodMs > 0)
+    SymbolsUpdatedSinceLastIndex = true;
+  else
----------------
kadircet wrote:
> why not simply check if `BuildIndexPeriodMs` has passed and issue rebuild here? That way we won't spawn an additional thread, and get rid of the CV
As chatted offline, the main problem is that the last index action might trigger an index update if it finishes shortly after an index rebuild; there is not trivial way for a index worker to figure out that it's the last run. Also the testing seems to be harder with this approach.

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55770/new/

https://reviews.llvm.org/D55770