ioeric added a comment. In https://reviews.llvm.org/D51475#1219197, @ilya-biryukov wrote:
> 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. I think the inconsistency is benign and should be worth 10 seconds (which for me personally is loong...). Besides, this is just LLVM index; the loading could be arbitrary long depending on the corpus size. ================ Comment at: clangd/tool/ClangdMain.cpp:86 + return nullptr; + Index = AsyncLoad.get(); + return Index.get(); ---------------- ilya-biryukov wrote: > 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; }); > ``` My concern with this approach is that we are still calling `wait` even though it's not necessary once Index has been loaded. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51475 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
