[clangd] Store paths as requested in PreambleStatCache

Underlying FS can store different file names inside the stat response
(e.g. symlinks resolved, absolute paths, dots removed). But we store path names
as requested inside the preamble,
https://github.com/llvm/llvm-project/blob/main/clang/lib/Serialization/ASTWriter.cpp#L1635.

This improves cache hit rates from ~30% to 90% in a build system that uses
symlinks.

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

Author: kadircet

clang-tools-extra/clangd/FS.cpp

@@ -11,6 +11,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include <optional>
+#include <utility>
 
 namespace clang {
 namespace clangd {
@@ -23,9 +24,10 @@ PreambleFileStatusCache::PreambleFileStatusCache(llvm::StringRef MainFilePath){
 }
 
 void PreambleFileStatusCache::update(const llvm::vfs::FileSystem &FS,
-                                     llvm::vfs::Status S) {
+                                     llvm::vfs::Status S,
+                                     llvm::StringRef File) {
   // Canonicalize path for later lookup, which is usually by absolute path.
-  llvm::SmallString<32> PathStore(S.getName());
+  llvm::SmallString<32> PathStore(File);
   if (FS.makeAbsolute(PathStore))
     return;
   llvm::sys::path::remove_dots(PathStore, /*remove_dot_dot=*/true);
@@ -72,14 +74,14 @@ PreambleFileStatusCache::getProducingFS(
       // many times (e.g. code completion) and the repeated status call is
       // likely to be cached in the underlying file system anyway.
       if (auto S = File->get()->status())
-        StatCache.update(getUnderlyingFS(), std::move(*S));
+        StatCache.update(getUnderlyingFS(), std::move(*S), Path.str());
       return File;
     }
 
     llvm::ErrorOr<llvm::vfs::Status> status(const llvm::Twine &Path) override {
       auto S = getUnderlyingFS().status(Path);
       if (S)
-        StatCache.update(getUnderlyingFS(), *S);
+        StatCache.update(getUnderlyingFS(), *S, Path.str());
       return S;
     }
 

clang-tools-extra/clangd/FS.h

@@ -41,7 +41,8 @@ class PreambleFileStatusCache {
   /// corresponds to. The stat for the main file will not be cached.
   PreambleFileStatusCache(llvm::StringRef MainFilePath);
 
-  void update(const llvm::vfs::FileSystem &FS, llvm::vfs::Status S);
+  void update(const llvm::vfs::FileSystem &FS, llvm::vfs::Status S,
+              llvm::StringRef File);
 
   /// \p Path is a path stored in preamble.
   std::optional<llvm::vfs::Status> lookup(llvm::StringRef Path) const;

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

@@ -37,18 +37,19 @@ TEST(FSTests, PreambleStatusCache) {
                       std::chrono::system_clock::now(), 0, 0, 1024,
                       llvm::sys::fs::file_type::regular_file,
                       llvm::sys::fs::all_all);
-  StatCache.update(*FS, S);
+  StatCache.update(*FS, S, "real");
   auto ConsumeFS = StatCache.getConsumingFS(FS);
-  auto Cached = ConsumeFS->status(testPath("fake"));
+  EXPECT_FALSE(ConsumeFS->status(testPath("fake")));
+  auto Cached = ConsumeFS->status(testPath("real"));
   EXPECT_TRUE(Cached);
-  EXPECT_EQ(Cached->getName(), testPath("fake"));
+  EXPECT_EQ(Cached->getName(), testPath("real"));
   EXPECT_EQ(Cached->getUniqueID(), S.getUniqueID());
 
-  // fake and temp/../fake should hit the same cache entry.
+  // real and temp/../real should hit the same cache entry.
   // However, the Status returned reflects the actual path requested.
-  auto CachedDotDot = ConsumeFS->status(testPath("temp/../fake"));
+  auto CachedDotDot = ConsumeFS->status(testPath("temp/../real"));
   EXPECT_TRUE(CachedDotDot);
-  EXPECT_EQ(CachedDotDot->getName(), testPath("temp/../fake"));
+  EXPECT_EQ(CachedDotDot->getName(), testPath("temp/../real"));
   EXPECT_EQ(CachedDotDot->getUniqueID(), S.getUniqueID());
 }