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

[Kadir Cetinkaya via Phabricator via cfe-commits]

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 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.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55315

Files:
  clangd/Threading.cpp
  clangd/Threading.h
  clangd/index/Background.cpp


Index: clangd/index/Background.cpp
===================================================================
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -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"
@@ -104,11 +105,6 @@
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
   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);
   }
 }
 
@@ -186,9 +182,17 @@
     auto *Storage = IndexStorageFactory(Project.SourceRoot);
     enqueueTask(Bind(
         [this, File, Storage](tooling::CompileCommand Cmd) {
+          // 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(ThreadPriority::Low);
           Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
           if (auto Error = index(std::move(Cmd), Storage))
             log("Indexing {0} failed: {1}", File, std::move(Error));
+          // Restore priority of the thread back to normal.
+          setThreadPriority(ThreadPriority::Normal);
         },
         std::move(*Cmd)));
   }
Index: clangd/Threading.h
===================================================================
--- clangd/Threading.h
+++ clangd/Threading.h
@@ -121,7 +121,8 @@
   Low = 0,
   Normal = 1,
 };
-void setThreadPriority(std::thread &T, ThreadPriority Priority);
+// Sets scheduling priority for the calling thread.
+void setThreadPriority(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.
Index: clangd/Threading.cpp
===================================================================
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -103,13 +103,13 @@
 
 static std::atomic<bool> AvoidThreadStarvation = {false};
 
-void setThreadPriority(std::thread &T, ThreadPriority Priority) {
+void setThreadPriority(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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55315.176794.patch
Type: text/x-patch
Size: 3188 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181205/93019fd8/attachment.bin>