[clangd] Add ability to change storage directory of index files

[crr0004]

Summary

This adds a new option to change the root directory of where the index files get stored.
I mainly added this because I am doing profiling of indexing and needed a way for multiple instances of index files not to conflict with each other.

The extact naming of variables and the help text can be changed, I just guessed.

I tried to add a test for the change however I couldn't find a way to neatly check storage root directory. I am not sure if the test I've written is all that helpful.

[kadircet]

I mainly added this because I am doing profiling of indexing and needed a way for multiple instances of index files not to conflict with each other.

If this is solely for testing purposes, can't you make use of --compile-commands-dir instead? You'll also need to copy your compile_commands.json into that directory.

[crr0004]

Strictly speaking you could, assuming you have a compile database and it's okay to have copies or symlinks of the database.

I would have probably done that if I had realised I could.

That does speak to another issue though; moving the compile database to move the index files is not very intuitive. We could improve the help text around compile commands to explain fully the implications of the file which might help.

I really just wanted direct control of where the index files go.

[kadircet]

Strictly speaking you could, assuming you have a compile database and it's okay to have copies or symlinks of the database.

Well you wouldn't have index shards without a compilation database anyway though.

That does speak to another issue though; moving the compile database to move the index files is not very intuitive. We could improve the help text around compile commands to explain fully the implications of the file which might help.

I am not sure if this is something that is interesting to the average user. Hence we've only documented in the designs and the code:

Before indexing each file, the index checks for a cached *.idx file on disk. After indexing, it writes this file. This avoids reindexing on startup if nothing changed since last time.
These files are located in .cache/clangd/index/ next to compile_commands.json. For headers with no CDB, such as the standard library, they are in clangd/index under the
user’s cache directory ($XDG_CACHE_HOME, DARWIN_USER_CACHE_DIR, or %LocalAppData%).

[sammccall]

This is definitely an interface that makes sense for the purpose you describe (profiling index performance) but I'm less convinced it makes sense for clangd's *primary* use cases.

For example, our default policy gives different answers for code in different paths, which has some desirable properties (e.g. once you stop working on a project, its index files can be removed in an obvious way without disturbing other projects).
A mechanism that allows customization but has its own limitations can add confusion without actually solving many more people's problems.
And generally we're trying to phase out use of global command line flags in favor of config files while carefully limiting the amount of options users need to understand.

Given that there are some workarounds available (compile-commands dir, making the .cache directory a symlink, maybe even using clangd-indexer instead depending on your purpose) I think we probably shouldn't add this option, or should at least defer it until we have more info on what kind of customization people need while using clangd together with an editor.

[crr0004]

Whoops. Hit close without adding this comment.

Okay thank you for that @sammccall and @kadirect. Ultimately shared this patch was I found it useful but wasn't sure if it would fit with the project.

Is there somewhere I can read about project direction/decisions? Like the stepping away from global config on the cli. Or is there a backlog of stuff that has been planned and is waiting for someone to pickup?

More than happy to continue to help with reviews, bugs and other day to day stuff.

clang-tools-extra/clangd/ClangdServer.h

struct Options {
     bool StorePreamblesInMemory = true;
 
     /// If true, ClangdServer builds a dynamic in-memory index for symbols in
     /// opened files and uses the index to augment code completion results.
     bool BuildDynamicSymbolIndex = false;
     /// If true, ClangdServer automatically indexes files in the current project
     /// on background threads. The index is stored in the project root.
     bool BackgroundIndex = false;
+  /// Path to force the disk storage of the index cache
+    llvm::Optional<std::string> BackgroundIndexCachePath;
 
     /// If set, use this index to augment code completion results.
     SymbolIndex *StaticIndex = nullptr;
 
     /// If set, queried to derive a processing context for some work.
     /// Usually used to inject Config (see createConfiguredContextProvider).
     ///
     /// When the provider is called, the active context will be that inherited

clang-tools-extra/clangd/ClangdServer.cpp

if (Opts.BackgroundIndex) {
     BGOpts.OnProgress = [Callbacks](BackgroundQueue::Stats S) {
       if (Callbacks)
         Callbacks->onBackgroundIndexProgress(S);
     };
     BGOpts.ContextProvider = Opts.ContextProvider;
     BackgroundIdx = std::make_unique<BackgroundIndex>(
         TFS, CDB,
         BackgroundIndexStorage::createDiskBackedStorageFactory(
-            [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
+            [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }, Opts.BackgroundIndexCachePath),
         std::move(BGOpts));
     AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)
     AddIndex(DynamicIdx.get());
 
   if (Opts.FeatureModules) {
     FeatureModule::Facilities F{

clang-tools-extra/clangd/index/Background.h

public:
   // itself. Factory must be threadsafe and never returns nullptr.
   using Factory = llvm::unique_function<BackgroundIndexStorage *(PathRef)>;
 
   // Creates an Index Storage that saves shards into disk. Index storage uses
   // CDBDirectory + ".cache/clangd/index/" as the folder to save shards.
   // CDBDirectory is the first directory containing a CDB in parent directories
   // of a file, or user cache directory if none was found, e.g. stdlib headers.
   static Factory createDiskBackedStorageFactory(
-      std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo);
+      std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo,
+      llvm::Optional<std::string> BackgroundIndexCachePath);
 };
 
 // A priority queue of tasks which can be run on (external) worker threads.
 class BackgroundQueue {
 public:
   /// A work item on the thread pool's queue.
   struct Task {
     explicit Task(std::function<void()> Run) : Run(std::move(Run)) {}

clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp

 };
 
 // Creates and owns IndexStorages for multiple CDBs.
 // When a CDB root is found, shards are stored in $ROOT/.cache/clangd/index/.
 // When no root is found, the fallback path is ~/.cache/clangd/index/.
 class DiskBackedIndexStorageManager {
 public:
   DiskBackedIndexStorageManager(
-      std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo)
+      std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo,
+      llvm::Optional<std::string> BackgroundDiskCachePath)
       : IndexStorageMapMu(std::make_unique<std::mutex>()),
-        GetProjectInfo(std::move(GetProjectInfo)) {
+        GetProjectInfo(std::move(GetProjectInfo)),
+        BackgroundDiskCachePath(BackgroundDiskCachePath) {
     llvm::SmallString<128> FallbackDir;
     if (llvm::sys::path::cache_directory(FallbackDir))
       llvm::sys::path::append(FallbackDir, "clangd", "index");
     this->FallbackDir = FallbackDir.str().str();
   }
 
   // Creates or fetches to storage from cache for the specified project.
   BackgroundIndexStorage *operator()(PathRef File) {
     std::lock_guard<std::mutex> Lock(*IndexStorageMapMu);
     llvm::SmallString<128> StorageDir(FallbackDir);
-    if (auto PI = GetProjectInfo(File)) {
+    if (BackgroundDiskCachePath &&
+        !BackgroundDiskCachePath.getValue().empty()) {
+      StorageDir = BackgroundDiskCachePath.getValue();
+      llvm::sys::path::append(StorageDir, ".cache", "clangd", "index");
+
+    } else if (auto PI = GetProjectInfo(File)) {
       StorageDir = PI->SourceRoot;
       llvm::sys::path::append(StorageDir, ".cache", "clangd", "index");
     }
     auto &IndexStorage = IndexStorageMap[StorageDir];
     if (!IndexStorage)
       IndexStorage = create(StorageDir);
     return IndexStorage.get();
   }
 
 private:
   std::unique_ptr<BackgroundIndexStorage> create(PathRef CDBDirectory) {
     if (CDBDirectory.empty()) {
       elog("Tried to create storage for empty directory!");
       return std::make_unique<NullStorage>();
     }
     return std::make_unique<DiskBackedIndexStorage>(CDBDirectory);
   }
 
   Path FallbackDir;
 
   llvm::StringMap<std::unique_ptr<BackgroundIndexStorage>> IndexStorageMap;
   std::unique_ptr<std::mutex> IndexStorageMapMu;
 
   std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo;
+  llvm::Optional<std::string> BackgroundDiskCachePath;
 };
 
 } // namespace
 
 BackgroundIndexStorage::Factory
 BackgroundIndexStorage::createDiskBackedStorageFactory(
-    std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo) {
-  return DiskBackedIndexStorageManager(std::move(GetProjectInfo));
+    std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo,
+    llvm::Optional<std::string> BackgroundIndexCachePath) {
+  return DiskBackedIndexStorageManager(std::move(GetProjectInfo),
+                                       std::move(BackgroundIndexCachePath));
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/tool/ClangdMain.cpp

desc(
         "Index file to build the static index. The file must have been created "
         "by a compatible clangd-indexer\n"
         "WARNING: This option is experimental only, and will be removed "
         "eventually. Don't rely on it"),
     init(""),
     Hidden,
 };
 
+opt<Path> BackgroundIndexCachePath{
+  "background-index-storage",
+  cat(Misc),
+  desc("The path to store the index files generated by background indexing"),
+  init(""),
+  Hidden
+};
+
 opt<bool> Test{
     "lit-test",
     cat(Misc),
     desc("Abbreviation for -input-style=delimited -pretty -sync "
          "-enable-test-scheme -enable-config=0 -log=verbose. "
          "Intended to simplify lit tests"),
     init(false),
     Hidden,
if (!RemoteIndexAddress.empty()) {
     } else {
       elog("When enabling remote index, IndexFile should not be specified. "
            "Only one can be used at time. Remote index will ignored.");
     }
   }
 #endif
   Opts.BackgroundIndex = EnableBackgroundIndex;
   Opts.ReferencesLimit = ReferencesLimit;
+  Opts.BackgroundIndexCachePath = BackgroundIndexCachePath;
   auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
   if (StaticIdx) {
     IdxStack.emplace_back(std::move(StaticIdx));
     IdxStack.emplace_back(
         std::make_unique<MergedIndex>(PAI.get(), IdxStack.back().get()));
     Opts.StaticIndex = IdxStack.back().get();
   } else {
     Opts.StaticIndex = PAI.get();

clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp

TEST_F(BackgroundIndexTest, Config) {
   }
   // Wait for both files to be indexed.
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   EXPECT_THAT(runFuzzyFind(Idx, ""),
               UnorderedElementsAre(QName("foo"), QName("foo::two"),
                                    QName("bar"), QName("bar::one")));
 }
 
+TEST_F(BackgroundIndexTest, StoreIndexedFilesInCustomPath){
+
+  OverlayCDB CDB(/*Base=*/nullptr);
+  auto DiskStorage = BackgroundIndexStorage::createDiskBackedStorageFactory(
+      [&CDB](llvm::StringRef File) {
+        return CDB.getProjectInfo(File);
+      },
+      testing::TempDir()
+  );
+
+  auto *Result = DiskStorage("root/A.cc");
+  IndexFileIn ShardIn{
+      SymbolSlab{},
+      RefSlab{},
+      RelationSlab{},
+      IncludeGraph{},
+      tooling::CompileCommand{}
+  };
+  IndexFileOut Shard{ShardIn};
+  auto Result2 = Result->storeShard("root/A.cc", Shard);
+
+  ASSERT_TRUE(Result);
+  ASSERT_FALSE(llvm::errorToBool(std::move(Result2)));
+}
 TEST_F(BackgroundIndexTest, IndexTwoFiles) {
   MockFS FS;
   // a.h yields different symbols when included by A.cc vs B.cc.
   FS.Files[testPath("root/A.h")] = R"cpp(
       void common();
       void f_b();
       #if A
         class A_CC {};