ioeric abandoned this revision.
ioeric added a comment.
Dropping this in favor of https://reviews.llvm.org/D51638
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li
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? L
ioeric added a comment.
In https://reviews.llvm.org/D51475#1219291, @sammccall wrote:
> I don't have a strong opinion on async vs sync - startup time is important
> and we shouldn't block simple AST-based functionality on the index, but this
> introduces some slightly confusing UX for that spee
ioeric added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:86
+ return nullptr;
+Index = AsyncLoad.get();
+return Index.get();
ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > ioeric wrote:
> > > > ilya-biryukov wrote:
>
sammccall added a comment.
(Sorry for catching this earlier, and that the patch isn't landed yet - feel
free to pick up the review, else @kbobyrev will take a first pass tomorrow I
think)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51475
_
sammccall added a comment.
I don't have a strong opinion on async vs sync - startup time is important and
we shouldn't block simple AST-based functionality on the index, but this
introduces some slightly confusing UX for that speed.
However I think this should be based on https://reviews.llvm.o
ilya-biryukov added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:86
+ return nullptr;
+Index = AsyncLoad.get();
+return Index.get();
ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > I believe is a d
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 c
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
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 clan
ioeric updated this revision to Diff 163321.
ioeric added a comment.
- Fix data race.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51475
Files:
clangd/tool/ClangdMain.cpp
Index: clangd/tool/ClangdMain.cpp
===
ilya-biryukov added a comment.
Any reason to not just wait for the index to load? Is this a UX concern or a
problem when experimenting?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51475
___
cfe-commits mailing list
cfe-commits@l
ilya-biryukov added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:86
+ return nullptr;
+Index = AsyncLoad.get();
+return Index.get();
I believe is a data race (multiple threads may run this line concurrently).
You would want some synchroniz
ioeric updated this revision to Diff 163276.
ioeric added a comment.
- revert unintended change
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51475
Files:
clangd/tool/ClangdMain.cpp
Index: clangd/tool/ClangdMain.cpp
==
14 matches
Mail list logo
