[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clangd/tool/ClangdMain.cpp:48
+// loading.
+class AsyncLoadIndex : public SymbolIndex {
+public:
----------------
Also, do we want only static index to be built asynchronously? Do we want it to be used only in our Clangd tool driver? Loading dynamic index might also be useful if we have this kind of behavior for the static one. I'm not completely sure about that, though.


================
Comment at: clangd/tool/ClangdMain.cpp:78
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-                : MemIndex::build(std::move(SymsBuilder).build());
+  size_t estimateMemoryUsage() const override { return 0; }
+
----------------
Shouldn't this `index()->estimateMemoryUsage()` when the index is built?


================
Comment at: clangd/tool/ClangdMain.cpp:102
+// that all symbols can be managed in memory.
+std::unique_ptr<SymbolIndex> buildStaticIndex(llvm::StringRef YamlSymbolFile) {
+  return llvm::make_unique<AsyncLoadIndex>(
----------------
Do we want to be more explicit about loading index asynchronously? If we're not that fact might be implicit to the user (e.g. "my Clangd is not frozen, but global completion doesn't work"), CLI flag/documentation entry might solve that issue. Introducing tons of not-so-useful flags is not optimal, though, and I'm also not sure about that.

What do you think?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475