[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

Fri Dec 7 05:24:08 PST 2018

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