ilya-biryukov updated this revision to Diff 132567.
ilya-biryukov added a comment.
Cleaned up the patch and added the missing bits.
This is in a much better shape now and should be ready for review.
I'll have a go through the existing review comments to make sure all concerns
were addressed.
Re
ilya-biryukov added inline comments.
Comment at: clangd/TUScheduler.h:63
+ Context Ctx, ParseInputs Inputs,
+ UniqueFunction>)>
+ OnUpdated);
sammccall wrote:
> do we want to move the callback to be a clangd-global thing, rather than a
> per-u
ilya-biryukov added inline comments.
Comment at: clangd/TUScheduler.cpp:220
+
+ GCThread.cleanupFile(std::move(Data->Worker));
}
sammccall wrote:
> in the spirit of "just spawn a thread, and write direct code"...
>
> can we just spawn a shared_ptr to do the wo
sammccall added a comment.
Not a complete review, but this looks pretty good!
You probably want to read the last comment first - the gc thread is the
threadpooliest bit of code left, and you may (or may not) want to eliminate it.
Comment at: clangd/TUScheduler.cpp:23
+retu
ilya-biryukov updated this revision to Diff 132105.
ilya-biryukov added a comment.
- An initial version of thread-per-file approach.
This is by no means a final version, we should definitely move things between
files, do some renames, etc. before landing the final version.
Some things are not us
sammccall added a comment.
OK, here's a braindump, probably nothing surprising after our offline
discussion.
TL;DR: the only design I *know* is extensible in ways we care about is the one
that basically shards Scheduler into a bunch of per-TU schedulers.
I think that's probably the way to go, b
sammccall added a comment.
Thanks, this helps me understand where previous patch is coming from!
I have some comments on the ThreadPool part, which basically amount to trying
to represent the same abstract structure with fewer pieces.
But I still want to consider whether that's the right structu
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.h:266
+
+ParseInputs Inputs;
+std::shared_ptr Resources;
These fields should probably be grouped into multiple groups:
- `Inputs` - capture the latest input. Can only be used on the main
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: cfe-commits, hintonda, ioeric, jkorous-apple, mgorny,
klimek.
DO NOT SUBMIT (yet). This is a non-final version of the patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D
