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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D51475#1219133, @ilya-biryukov wrote:

> Any reason to not just wait for the index to load? Is this a UX concern or a problem when experimenting?


The index loading can be slow. When using LLVM YAML index, I need to wait for >10s before clangd starts giving me anything useful. We could potentially speed up loading (e.g. replacing yaml), but the index can be arbitrary large. I think it's an improvement in general to be able to get clangd running before index is loaded.



================
Comment at: clangd/tool/ClangdMain.cpp:86
+      return nullptr;
+    Index = AsyncLoad.get();
+    return Index.get();
----------------
ilya-biryukov wrote:
> I believe is a data race (multiple threads may run this line concurrently).
> You would want some synchronization around this, `std::shared_future` could be a good fit
Nice catch. 

I am a bit hesitated about using `shared_future` though. It could potentially get rid of the mutex but would require much more careful use. For example, `Index` can be set to the same value repeatedly, which makes me a bit nervous.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475