kadircet added inline comments.
================
Comment at: clangd/index/Background.cpp:202
std::lock_guard<std::mutex> Lock(QueueMu);
- Queue.push_back(std::move(T));
+ if (Priority == ThreadPriority::Low) {
+ Queue.push_back(Bind(
----------------
ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > Since we might be interested in scheduling higher-priority tasks first
> > > anyway (not in this patch, but still), maybe store a pair of `(Task,
> > > Priority)` in the queue and call `setCurrentThreadPriority` when actually
> > > running the task?
> > I believe we can introduce that later on whenever we have more than 2
> > priorities, currently we push to front for important tasks and back for the
> > low priority ones. Do you think it is not enough?
> Sorry, I somehow missed that `push_front` and `push_back` are used in
> different cases, I've only noticed the callback wrapping.
> I feel storing priorities in the queue produces a more straightforward code
> as priority is an important scheduling signal, so it fits in nicely there.
> Using a wrapped callback, OTOH, is a bit hard to read. But that might be a
> personal preference, up to you.
>
> See the other comment about ordering of the tasks with normal priorities, it
> looks more important
The wrapped callback was rather for getting rid of unnecessary system calls,
but since normal tasks come up rarely to queue it doesn't feel like an
important thing. Changing it to store the priority in the queue.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55315/new/
https://reviews.llvm.org/D55315
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
