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

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 30 06:53:35 PDT 2018

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.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list