This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE349345: [clangd] Only reduce priority of a thread for
indexing. (authored by kadircet, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D55315?vs=178443&id=178445#toc
Repository:
r
kadircet updated this revision to Diff 178443.
kadircet marked an inline comment as done.
kadircet added a comment.
- Revert name from Tasks to Queue
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55315/new/
https://reviews.llvm.org/D55315
Files:
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: clangd/index/Background.h:121
bool ShouldStop = false;
- std::deque Queue;
+ std::deque> Tasks;
std::vector ThreadPool; // FIXME: Abstr
kadircet updated this revision to Diff 178094.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Use std::deque instead of std::list
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55315/new/
https://reviews.llvm.org/D55315
Files
kadircet added inline comments.
Comment at: clangd/index/Background.h:121
bool ShouldStop = false;
- std::deque Queue;
+ std::list> Tasks;
std::vector ThreadPool; // FIXME: Abstract this away.
ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote
ilya-biryukov added inline comments.
Comment at: clangd/index/Background.h:121
bool ShouldStop = false;
- std::deque Queue;
+ std::list> Tasks;
std::vector ThreadPool; // FIXME: Abstract this away.
kadircet wrote:
> ilya-biryukov wrote:
> > Why not `std::
kadircet marked an inline comment as done.
kadircet added inline comments.
Comment at: clangd/index/Background.h:121
bool ShouldStop = false;
- std::deque Queue;
+ std::list> Tasks;
std::vector ThreadPool; // FIXME: Abstract this away.
ilya-biryukov wrote
ilya-biryukov added inline comments.
Comment at: clangd/index/Background.h:121
bool ShouldStop = false;
- std::deque Queue;
+ std::list> Tasks;
std::vector ThreadPool; // FIXME: Abstract this away.
Why not `std::deque`?
Repository:
rCTE Clang Tools Ex
kadircet updated this revision to Diff 178052.
kadircet marked 6 inline comments as done.
kadircet added a comment.
- Address comments
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55315/new/
https://reviews.llvm.org/D55315
Files:
clangd/Threadin
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/Backgroun
kadircet updated this revision to Diff 177188.
kadircet marked 3 inline comments as done.
kadircet added a comment.
- Move priority change logic to runner
- Change storage for tasks from deque to list, since we only pop from front but
we might end up adding into middle.
Repository:
rCTE Clang
kadircet added inline comments.
Comment at: clangd/index/Background.cpp:202
std::lock_guard Lock(QueueMu);
-Queue.push_back(std::move(T));
+if (Priority == ThreadPriority::Low) {
+ Queue.push_back(Bind(
ilya-biryukov wrote:
> kadircet wrote:
> >
ilya-biryukov added inline comments.
Comment at: clangd/index/Background.cpp:202
std::lock_guard Lock(QueueMu);
-Queue.push_back(std::move(T));
+if (Priority == ThreadPriority::Low) {
+ Queue.push_back(Bind(
kadircet wrote:
> ilya-biryukov wrote
kadircet added inline comments.
Comment at: clangd/index/Background.cpp:202
std::lock_guard Lock(QueueMu);
-Queue.push_back(std::move(T));
+if (Priority == ThreadPriority::Low) {
+ Queue.push_back(Bind(
ilya-biryukov wrote:
> Since we might be i
kadircet updated this revision to Diff 177007.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Delete redundant comment
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55315/new/
https://reviews.llvm.org/D55315
Files:
clangd/
ilya-biryukov added inline comments.
Comment at: clangd/Threading.h:124
};
-void setThreadPriority(std::thread &T, ThreadPriority Priority);
+// Sets scheduling priority for the calling thread.
+void setCurrentThreadPriority(ThreadPriority Priority);
The comment
kadircet updated this revision to Diff 176834.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Move priority change logic into enqueueTask
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55315/new/
https://reviews.llvm.org/D5531
ilya-biryukov added inline comments.
Comment at: clangd/Threading.h:125
+// Sets scheduling priority for the calling thread.
+void setThreadPriority(ThreadPriority Priority);
// Avoid the use of scheduler policies that may starve low-priority threads.
Maybe chan
kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay, ioeric.
We'll soon have tasks pending for reading shards from disk, we want
them to have normal priority. Because:
- They are not CPU intensive, mostly
19 matches
Mail list logo
