ilya-biryukov added inline comments.
================
Comment at: clangd/ClangdServer.h:177
+/// preambles and ASTs) for opened files.
+class Scheduler {
+public:
----------------
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?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42174
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
