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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/index/Background.cpp:107
   while (ThreadPoolSize--) {
     ThreadPool.emplace_back([this] { run(); });
   }
----------------
NIT: remove braces around a single statement


================
Comment at: clangd/index/Background.cpp:142
     }
+    setCurrentThreadPriority(Priority);
     (*Task)();
----------------
NIT: maybe add newlines around the three lines (`setCurrentThreadPriority` and Task execution). These are pretty important.
 


================
Comment at: clangd/index/Background.cpp:142
     }
+    setCurrentThreadPriority(Priority);
     (*Task)();
----------------
ilya-biryukov wrote:
> NIT: maybe add newlines around the three lines (`setCurrentThreadPriority` and Task execution). These are pretty important.
>  
Maybe only lower the priority iff it's `!= Normal`. Would avoid system calls in most cases (I believe you mentioned it in the other comment before too)


================
Comment at: clangd/index/Background.cpp:206
+    auto I = Tasks.end();
+    if (Priority == ThreadPriority::Normal) {
+      I = Tasks.begin();
----------------
A short comment about the structure of the queue would be in order, i.e. mention that we first store all "normal" tasks, followed by "low" tasks. And that we don't expect the number of "normal" tasks to grow beyond single-digit numbers, so it's ok to do linear search here and insert in that position


================
Comment at: clangd/index/Background.cpp:208
+      I = Tasks.begin();
+      while (I->second == ThreadPriority::Normal)
+        I++;
----------------
Maybe change to `llvm::find_if(Tasks, [](Task &T) { return T.second == ThreadPriority::Low); } `?
More concise and avoids explicit loop.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55315/new/

https://reviews.llvm.org/D55315