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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D51475#1219184, @ioeric wrote:

> 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.


I would trade-off those 10 seconds for giving consistent experience (i.e. avoiding confusing the users with different modes of completion (with and without index, etc.)).
But not terribly opposed to that.



================
Comment at: clangd/tool/ClangdMain.cpp:86
+      return nullptr;
+    Index = AsyncLoad.get();
+    return Index.get();
----------------
ioeric wrote:
> 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.
The following pattern should give proper results:
```
class AsyncIndex : public Index {
public:
  AsyncIndex(std::shared_future<Index> IndexFut);
  //....
private;
  const SymbolIndex *index() const {
     if (IndexFut.wait(0) != ready)
       return nullptr;
     return &IndexFut.get();
  }

  std::shared_future<Index>IndexFut;
};


AsyncIndex AI(std::async([]() { /* load the index here */ return Index; });
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475