This revision was automatically updated to reflect the committed changes.
Closed by commit rL324356: [clangd] The new threading implementation (authored
by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D42573
Files:
ilya-biryukov updated this revision to Diff 133003.
ilya-biryukov added a comment.
- Addressed the last review comment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42573
Files:
clangd/CMakeLists.txt
clangd/ClangdServer.h
clangd/ClangdUnit.h
clangd/ClangdUnitStore.cpp
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
This is really great. Just one test nit.
Comment at: unittests/clangd/ThreadingTests.cpp:34
+
+scheduleIncrements();
+Tasks.waitForAll();
The
ilya-biryukov updated this revision to Diff 132967.
ilya-biryukov marked 21 inline comments as done.
ilya-biryukov added a comment.
- Renamed File to AST.
- Introduced startTask().
- Moved small methods of ASTWorkerHandle to have inline definitions.
- Removed constructor of FileData.
- Replaced
ilya-biryukov added inline comments.
Comment at: clangd/ASTWorker.cpp:102
+ // not waste time on it.
+ LastUpdateCF->cancel();
+}
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > This strategy has some upsides:
> > > - we
sammccall added a comment.
Very nice! Thanks for adding the docs to TUScheduler implementation, makes a
big difference.
Rest is almost all readability/comment bits. Substantive stuff is:
- getUsedBytes() looks racy
- I'm not sure we're choosing the right preamble
My understanding is the
ilya-biryukov updated this revision to Diff 132821.
ilya-biryukov marked 13 inline comments as done.
ilya-biryukov added a comment.
- Removed ASTWorker files, moved all the code to TUScheduler.cpp
- Renamed setDone to stop
- Added a comment to TUScheduler.cpp
- Addressed other review comments
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.
Comment at: clangd/ASTWorker.cpp:1
+//===--- ASTWorker.cpp
*-C++-*-===//
+//
sammccall wrote:
> This file could really use some
ilya-biryukov updated this revision to Diff 132803.
ilya-biryukov added a comment.
- Changed interface of ASTWorker so that it runs the processing loop itself.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42573
Files:
clangd/ASTWorker.cpp
clangd/ASTWorker.h
sammccall added a comment.
Really coming together!
Comment at: clangd/ASTWorker.cpp:1
+//===--- ASTWorker.cpp
*-C++-*-===//
+//
This file could really use some high-level comments about the scheduling
strategy,
ilya-biryukov updated this revision to Diff 132570.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Removed redundant includes
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42573
Files:
clangd/ASTWorker.cpp
clangd/ASTWorker.h
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.h:140
+/// the working threads as soon as an idle thread is available.
+///- scheduleOnQueue will schedule to a specific queue. Requests from the
+///
12 matches
Mail list logo