shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[clangd] Store paths as requested in PreambleStatCache

Browse Source 
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
This commit is contained in:
Kadir Cetinkaya 2023-05-23 09:47:57 +02:00
parent 70688e82e2
commit 35ce741ef3
No known key found for this signature in database
GPG Key ID: E39E36B8D2057ED6
3 changed files with 15 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
clang-tools-extra/clangd/FS.cpp
View File

@ -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;
}

3
clang-tools-extra/clangd/FS.h
View File

@ -41,7 +41,8 @@ public:
/// 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;

13
clang-tools-extra/clangd/unittests/FSTests.cpp
View File

@ -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());
}