[PATCH] D125673: [clangd] Add command-line flag to set background indexing thread priority.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This is a followup to D124715 <https://reviews.llvm.org/D124715>, which changed the default, and it anticipates
future patches raising the priority of Low (which is currently equal to
Background on Windows & Linux).
The main point is to allow users to restore the old behavior, which e.g.
allows efficiency cores to remain idle.

I did consider making this a config setting, this is a more complicated change:

- needs to touch queue priorities as well as thread priorities
- we don't know the priority until evaluating the config inside the task
- users would want the ability to prioritize background indexing tasks relative to each other without necessarily affecting thread priority, so using one option for both may be confusing

I don't really have a use case, so I prefer the simpler thing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125673

Files:
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -169,6 +169,21 @@
     init(true),
 };
 
+opt<llvm::ThreadPriority> BackgroundIndexPriority{
+    "background-index-priority",
+    cat(Features),
+    desc("Thread priority for building the background index. "
+         "The effect of this flag is OS-specific."),
+    values(clEnumValN(llvm::ThreadPriority::Background, "background",
+                      "Minimum priority, runs on idle CPUs. "
+                      "May leave 'performance' cores unused."),
+           clEnumValN(llvm::ThreadPriority::Low, "low",
+                      "Reduced priority compared to interactive work."),
+           clEnumValN(llvm::ThreadPriority::Default, "normal",
+                      "Same priority as other clangd work.")),
+    init(llvm::ThreadPriority::Low),
+};
+
 opt<bool> EnableClangTidy{
     "clang-tidy",
     cat(Features),
@@ -868,6 +883,7 @@
   }
 #endif
   Opts.BackgroundIndex = EnableBackgroundIndex;
+  Opts.BackgroundIndexPriority = BackgroundIndexPriority;
   Opts.ReferencesLimit = ReferencesLimit;
   auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
   if (StaticIdx) {
Index: clang-tools-extra/clangd/index/Background.h
===================================================================
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -136,6 +136,8 @@
     // Arbitrary value to ensure some concurrency in tests.
     // In production an explicit value is specified.
     size_t ThreadPoolSize = 4;
+    // Thread priority when indexing files.
+    llvm::ThreadPriority IndexingPriority = llvm::ThreadPriority::Low;
     // Callback that provides notifications as indexing makes progress.
     std::function<void(BackgroundQueue::Stats)> OnProgress = nullptr;
     // Function called to obtain the Context to use while indexing the specified
@@ -194,6 +196,7 @@
   // configuration
   const ThreadsafeFS &TFS;
   const GlobalCompilationDatabase &CDB;
+  llvm::ThreadPriority IndexingPriority;
   std::function<Context(PathRef)> ContextProvider;
 
   llvm::Error index(tooling::CompileCommand);
Index: clang-tools-extra/clangd/index/Background.cpp
===================================================================
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -92,6 +92,7 @@
     const ThreadsafeFS &TFS, const GlobalCompilationDatabase &CDB,
     BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts)
     : SwapIndex(std::make_unique<MemIndex>()), TFS(TFS), CDB(CDB),
+      IndexingPriority(Opts.IndexingPriority),
       ContextProvider(std::move(Opts.ContextProvider)),
       IndexedSymbols(IndexContents::All),
       Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize),
@@ -165,6 +166,7 @@
       elog("Indexing {0} failed: {1}", Path, std::move(Error));
   });
   T.QueuePri = IndexFile;
+  T.ThreadPri = IndexingPriority;
   T.Tag = std::move(Tag);
   T.Key = Key;
   return T;
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -110,6 +110,7 @@
     /// If true, ClangdServer automatically indexes files in the current project
     /// on background threads. The index is stored in the project root.
     bool BackgroundIndex = false;
+    llvm::ThreadPriority BackgroundIndexPriority = llvm::ThreadPriority::Low;
 
     /// If set, use this index to augment code completion results.
     SymbolIndex *StaticIndex = nullptr;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125673.429655.patch
Type: text/x-patch
Size: 3770 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220516/27472715/attachment-0001.bin>