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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
