[clang-tools-extra] r349345 - [clangd] Only reduce priority of a thread for indexing.

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 17 04:30:27 PST 2018


Author: kadircet
Date: Mon Dec 17 04:30:27 2018
New Revision: 349345

URL: http://llvm.org/viewvc/llvm-project?rev=349345&view=rev
Log:
[clangd] Only reduce priority of a thread for indexing.

Summary:
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 IO bound.
- Give a good coverage for the project at startup, therefore it is worth
  spending some cycles.
- We have only one task per whole CDB rather than one task per file.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, jfb, cfe-commits

Differential Revision: https://reviews.llvm.org/D55315

Modified:
    clang-tools-extra/trunk/clangd/Threading.cpp
    clang-tools-extra/trunk/clangd/Threading.h
    clang-tools-extra/trunk/clangd/index/Background.cpp
    clang-tools-extra/trunk/clangd/index/Background.h

Modified: clang-tools-extra/trunk/clangd/Threading.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.cpp?rev=349345&r1=349344&r2=349345&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Threading.cpp (original)
+++ clang-tools-extra/trunk/clangd/Threading.cpp Mon Dec 17 04:30:27 2018
@@ -112,13 +112,13 @@ void wait(std::unique_lock<std::mutex> &
 
 static std::atomic<bool> AvoidThreadStarvation = {false};
 
-void setThreadPriority(std::thread &T, ThreadPriority Priority) {
+void setCurrentThreadPriority(ThreadPriority Priority) {
   // Some *really* old glibcs are missing SCHED_IDLE.
 #if defined(__linux__) && defined(SCHED_IDLE)
   sched_param priority;
   priority.sched_priority = 0;
   pthread_setschedparam(
-      T.native_handle(),
+      pthread_self(),
       Priority == ThreadPriority::Low && !AvoidThreadStarvation ? SCHED_IDLE
                                                                 : SCHED_OTHER,
       &priority);

Modified: clang-tools-extra/trunk/clangd/Threading.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.h?rev=349345&r1=349344&r2=349345&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Threading.h (original)
+++ clang-tools-extra/trunk/clangd/Threading.h Mon Dec 17 04:30:27 2018
@@ -122,7 +122,7 @@ enum class ThreadPriority {
   Low = 0,
   Normal = 1,
 };
-void setThreadPriority(std::thread &T, ThreadPriority Priority);
+void setCurrentThreadPriority(ThreadPriority Priority);
 // Avoid the use of scheduler policies that may starve low-priority threads.
 // This prevents tests from timing out on loaded systems.
 // Affects subsequent setThreadPriority() calls.

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349345&r1=349344&r2=349345&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Dec 17 04:30:27 2018
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
@@ -135,14 +136,8 @@ BackgroundIndex::BackgroundIndex(
           })) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  while (ThreadPoolSize--) {
+  while (ThreadPoolSize--)
     ThreadPool.emplace_back([this] { run(); });
-    // Set priority to low, since background indexing is a long running task we
-    // do not want to eat up cpu when there are any other high priority threads.
-    // FIXME: In the future we might want a more general way of handling this to
-    // support tasks with various priorities.
-    setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
-  }
 }
 
 BackgroundIndex::~BackgroundIndex() {
@@ -163,6 +158,7 @@ void BackgroundIndex::run() {
   WithContext Background(BackgroundContext.clone());
   while (true) {
     Optional<Task> Task;
+    ThreadPriority Priority;
     {
       std::unique_lock<std::mutex> Lock(QueueMu);
       QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
@@ -172,10 +168,16 @@ void BackgroundIndex::run() {
         return;
       }
       ++NumActiveTasks;
-      Task = std::move(Queue.front());
+      std::tie(Task, Priority) = std::move(Queue.front());
       Queue.pop_front();
     }
+
+    if (Priority != ThreadPriority::Normal)
+      setCurrentThreadPriority(Priority);
     (*Task)();
+    if (Priority != ThreadPriority::Normal)
+      setCurrentThreadPriority(ThreadPriority::Normal);
+
     {
       std::unique_lock<std::mutex> Lock(QueueMu);
       assert(NumActiveTasks > 0 && "before decrementing");
@@ -193,44 +195,60 @@ bool BackgroundIndex::blockUntilIdleForT
 }
 
 void BackgroundIndex::enqueue(const std::vector<std::string> &ChangedFiles) {
-  enqueueTask([this, ChangedFiles] {
-    trace::Span Tracer("BackgroundIndexEnqueue");
-    // We're doing this asynchronously, because we'll read shards here too.
-    // FIXME: read shards here too.
-
-    log("Enqueueing {0} commands for indexing", ChangedFiles.size());
-    SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
-
-    // We shuffle the files because processing them in a random order should
-    // quickly give us good coverage of headers in the project.
-    std::vector<unsigned> Permutation(ChangedFiles.size());
-    std::iota(Permutation.begin(), Permutation.end(), 0);
-    std::mt19937 Generator(std::random_device{}());
-    std::shuffle(Permutation.begin(), Permutation.end(), Generator);
-
-    for (const unsigned I : Permutation)
-      enqueue(ChangedFiles[I]);
-  });
+  enqueueTask(
+      [this, ChangedFiles] {
+        trace::Span Tracer("BackgroundIndexEnqueue");
+        // We're doing this asynchronously, because we'll read shards here too.
+        // FIXME: read shards here too.
+
+        log("Enqueueing {0} commands for indexing", ChangedFiles.size());
+        SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
+
+        // We shuffle the files because processing them in a random order should
+        // quickly give us good coverage of headers in the project.
+        std::vector<unsigned> Permutation(ChangedFiles.size());
+        std::iota(Permutation.begin(), Permutation.end(), 0);
+        std::mt19937 Generator(std::random_device{}());
+        std::shuffle(Permutation.begin(), Permutation.end(), Generator);
+
+        for (const unsigned I : Permutation)
+          enqueue(ChangedFiles[I]);
+      },
+      ThreadPriority::Normal);
 }
 
 void BackgroundIndex::enqueue(const std::string &File) {
   ProjectInfo Project;
   if (auto Cmd = CDB.getCompileCommand(File, &Project)) {
     auto *Storage = IndexStorageFactory(Project.SourceRoot);
+    // Set priority to low, since background indexing is a long running
+    // task we do not want to eat up cpu when there are any other high
+    // priority threads.
     enqueueTask(Bind(
-        [this, File, Storage](tooling::CompileCommand Cmd) {
-          Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
-          if (auto Error = index(std::move(Cmd), Storage))
-            log("Indexing {0} failed: {1}", File, std::move(Error));
-        },
-        std::move(*Cmd)));
+                    [this, File, Storage](tooling::CompileCommand Cmd) {
+                      Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
+                      if (auto Error = index(std::move(Cmd), Storage))
+                        log("Indexing {0} failed: {1}", File, std::move(Error));
+                    },
+                    std::move(*Cmd)),
+                ThreadPriority::Low);
   }
 }
 
-void BackgroundIndex::enqueueTask(Task T) {
+void BackgroundIndex::enqueueTask(Task T, ThreadPriority Priority) {
   {
     std::lock_guard<std::mutex> Lock(QueueMu);
-    Queue.push_back(std::move(T));
+    auto I = Queue.end();
+    // We first store the tasks with Normal priority in the front of the queue.
+    // Then we store low priority tasks. Normal priority tasks are pretty rare,
+    // they should not grow beyond single-digit numbers, so it is OK to do
+    // linear search and insert after that.
+    if (Priority == ThreadPriority::Normal) {
+      I = llvm::find_if(Queue, [](const std::pair<Task, ThreadPriority> &Elem) {
+        return Elem.second == ThreadPriority::Low;
+      });
+    }
+    Queue.insert(I, {std::move(T), Priority});
   }
   QueueCV.notify_all();
 }

Modified: clang-tools-extra/trunk/clangd/index/Background.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.h?rev=349345&r1=349344&r2=349345&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h (original)
+++ clang-tools-extra/trunk/clangd/index/Background.h Mon Dec 17 04:30:27 2018
@@ -13,6 +13,7 @@
 #include "Context.h"
 #include "FSProvider.h"
 #include "GlobalCompilationDatabase.h"
+#include "Threading.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
@@ -110,14 +111,14 @@ private:
   // queue management
   using Task = std::function<void()>;
   void run(); // Main loop executed by Thread. Runs tasks from Queue.
-  void enqueueTask(Task T);
+  void enqueueTask(Task T, ThreadPriority Prioirty);
   void enqueueLocked(tooling::CompileCommand Cmd,
                      BackgroundIndexStorage *IndexStorage);
   std::mutex QueueMu;
   unsigned NumActiveTasks = 0; // Only idle when queue is empty *and* no tasks.
   std::condition_variable QueueCV;
   bool ShouldStop = false;
-  std::deque<Task> Queue;
+  std::deque<std::pair<Task, ThreadPriority>> Queue;
   std::vector<std::thread> ThreadPool; // FIXME: Abstract this away.
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
 };




More information about the cfe-commits mailing list