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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
