[PATCH] D42174: [clangd] Refactored threading in ClangdServer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: hokein.
sammccall added inline comments.


================
Comment at: clangd/ClangdServer.h:177
+/// preambles and ASTs) for opened files.
+class Scheduler {
+public:
----------------
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > sammccall wrote:
> > > > > This class has important responsibilities beyond threading itself, which "Scheduler" suggests.
> > > > > 
> > > > > I can't think of a perfectly coherent name, options that seem reasonable:
> > > > >  - TUManager - pretty bland/vague, but gets what this class is mostly about
> > > > >  - Workshop - kind of a silly metaphor, but unlikely to be confused with something else
> > > > >  - Clearinghouse - another silly metaphor, maybe more accurate but more obscure
> > > > Worth saying something abouth the threading properties here:
> > > > 
> > > > - Scheduler is not threadsafe, only the main thread should be providing updates and scheduling tasks.
> > > >  - callbacks are run on a large threadpool, and it's appropriate to do slow, blocking work in them
> > > Added comments.
> > It'd be nice to have some mention of the fact that the class handles threading responsibilities. None of the options seem to capture this.
> > I don't have good suggestions either, though.
> > 
> Rename of the Scheduler seems to be the only thing blocking this patch from landing.
> I'm happy to go with either of the suggested alternatives or leave as is, I couldn't come up with anything better.
> 
> @sammccall, what option would you prefer?
For a descriptive name I think we need something TU-related in there. If we want it to cover threads too, I can't think of anything better than `TUScheduler`.

I'm also happy with `Workshop` or similar, which prods you to work out what the class actually does. Maybe other people will find this confusing. Will solicit opinions.

...

Chatted with @ioeric and @hokein - Workshop is too silly. Suggestions were along the lines of what we've been discussing: TUTaskManager or similar.
So I'd probably go with TUScheduler, it's not horrendously long and it's close enough to Scheduler that those of us who spent last week discussing it won't get confused :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42174