Index: clangd/CMakeLists.txt =================================================================== --- clangd/CMakeLists.txt +++ clangd/CMakeLists.txt @@ -21,6 +21,7 @@ DraftStore.cpp FindSymbols.cpp FileDistance.cpp + FS.cpp FuzzyMatch.cpp GlobalCompilationDatabase.cpp Headers.cpp Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -181,8 +181,6 @@ if (isCancelled()) return CB(llvm::make_error()); - auto PreambleData = IP->Preamble; - llvm::Optional SpecFuzzyFind; if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) { SpecFuzzyFind.emplace(); @@ -195,10 +193,8 @@ // FIXME(ibiryukov): even if Preamble is non-null, we may want to check // both the old and the new version in case only one of them matches. CodeCompleteResult Result = clangd::codeComplete( - File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr, - PreambleData ? PreambleData->Includes : IncludeStructure(), - IP->Contents, Pos, FS, PCHs, CodeCompleteOpts, - SpecFuzzyFind ? SpecFuzzyFind.getPointer() : nullptr); + File, IP->Command, IP->Preamble, IP->Contents, Pos, FS, PCHs, + CodeCompleteOpts, SpecFuzzyFind ? SpecFuzzyFind.getPointer() : nullptr); { clang::clangd::trace::Span Tracer("Completion results callback"); CB(std::move(Result)); @@ -231,9 +227,8 @@ return CB(IP.takeError()); auto PreambleData = IP->Preamble; - CB(clangd::signatureHelp(File, IP->Command, - PreambleData ? &PreambleData->Preamble : nullptr, - IP->Contents, Pos, FS, PCHs, Index)); + CB(clangd::signatureHelp(File, IP->Command, PreambleData, IP->Contents, Pos, + FS, PCHs, Index)); }; // Unlike code completion, we wait for an up-to-date preamble here. Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H #include "Diagnostics.h" +#include "FS.h" #include "Function.h" #include "Headers.h" #include "Path.h" @@ -45,7 +46,8 @@ // Stores Preamble and associated data. struct PreambleData { PreambleData(PrecompiledPreamble Preamble, std::vector Diags, - IncludeStructure Includes); + IncludeStructure Includes, + std::unique_ptr StatCache); tooling::CompileCommand CompileCommand; PrecompiledPreamble Preamble; @@ -53,6 +55,9 @@ // Processes like code completions and go-to-definitions will need #include // information, and their compile action skips preamble range. IncludeStructure Includes; + // Cache of FS operations performed when building the preamble. + // When reusing a preamble, this cache can be consumed to save IO. + std::unique_ptr StatCache; }; /// Information required to run clang, e.g. to parse AST or do code completion. Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -246,9 +246,10 @@ } PreambleData::PreambleData(PrecompiledPreamble Preamble, - std::vector Diags, IncludeStructure Includes) + std::vector Diags, IncludeStructure Includes, + std::unique_ptr StatCache) : Preamble(std::move(Preamble)), Diags(std::move(Diags)), - Includes(std::move(Includes)) {} + Includes(std::move(Includes)), StatCache(std::move(StatCache)) {} ParsedAST::ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, @@ -334,9 +335,12 @@ // We proceed anyway, our lit-tests rely on results for non-existing working // dirs. } + + auto StatCache = llvm::make_unique(); auto BuiltPreamble = PrecompiledPreamble::Build( - CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Inputs.FS, PCHs, - StoreInMemory, SerializedDeclsCollector); + CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, + StatCache->getProducingFS(Inputs.FS), PCHs, StoreInMemory, + SerializedDeclsCollector); // When building the AST for the main file, we do want the function // bodies. @@ -347,7 +351,7 @@ FileName); return std::make_shared( std::move(*BuiltPreamble), PreambleDiagnostics.take(), - SerializedDeclsCollector.takeIncludes()); + SerializedDeclsCollector.takeIncludes(), std::move(StatCache)); } else { elog("Could not build a preamble for file {0}", FileName); return nullptr; @@ -361,15 +365,19 @@ trace::Span Tracer("BuildAST"); SPAN_ATTACH(Tracer, "File", FileName); - if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) { + auto VFS = Inputs.FS; + if (Preamble && Preamble->StatCache) + VFS = Preamble->StatCache->getConsumingFS(std::move(VFS)); + if (VFS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) { log("Couldn't set working directory when building the preamble."); // We proceed anyway, our lit-tests rely on results for non-existing working // dirs. } - return ParsedAST::build( - llvm::make_unique(*Invocation), Preamble, - llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), PCHs, Inputs.FS); + return ParsedAST::build(llvm::make_unique(*Invocation), + Preamble, + llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), + PCHs, std::move(VFS)); } SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit, Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -16,6 +16,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H +#include "ClangdUnit.h" #include "Headers.h" #include "Logger.h" #include "Path.h" @@ -221,8 +222,7 @@ /// destroyed when the async request finishes. CodeCompleteResult codeComplete(PathRef FileName, const tooling::CompileCommand &Command, - PrecompiledPreamble const *Preamble, - const IncludeStructure &PreambleInclusions, + const PreambleData *Preamble, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, @@ -232,8 +232,8 @@ /// Get signature help at a specified \p Pos in \p FileName. SignatureHelp signatureHelp(PathRef FileName, const tooling::CompileCommand &Command, - PrecompiledPreamble const *Preamble, - StringRef Contents, Position Pos, + const PreambleData *Preamble, StringRef Contents, + Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, const SymbolIndex *Index); Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -20,6 +20,7 @@ #include "CodeComplete.h" #include "AST.h" +#include "ClangdUnit.h" #include "CodeCompletionStrings.h" #include "Compiler.h" #include "Diagnostics.h" @@ -986,7 +987,7 @@ struct SemaCompleteInput { PathRef FileName; const tooling::CompileCommand &Command; - PrecompiledPreamble const *Preamble; + const PreambleData *Preamble; StringRef Contents; Position Pos; IntrusiveRefCntPtr VFS; @@ -1010,12 +1011,15 @@ // working dirs. } + IntrusiveRefCntPtr VFS = Input.VFS; + if (Input.Preamble && Input.Preamble->StatCache) + VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS)); IgnoreDiagnostics DummyDiagsConsumer; auto CI = createInvocationFromCommandLine( ArgStrs, CompilerInstance::createDiagnostics(new DiagnosticOptions, &DummyDiagsConsumer, false), - Input.VFS); + VFS); if (!CI) { elog("Couldn't create CompilerInvocation"); return false; @@ -1054,8 +1058,10 @@ // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise // the remapped buffers do not get freed. auto Clang = prepareCompilerInstance( - std::move(CI), CompletingInPreamble ? nullptr : Input.Preamble, - std::move(ContentsBuffer), std::move(Input.PCHs), std::move(Input.VFS), + std::move(CI), + (Input.Preamble && !CompletingInPreamble) ? &Input.Preamble->Preamble + : nullptr, + std::move(ContentsBuffer), std::move(Input.PCHs), std::move(VFS), DummyDiagsConsumer); Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble; Clang->setCodeCompletionConsumer(Consumer.release()); @@ -1565,19 +1571,20 @@ CodeCompleteResult codeComplete(PathRef FileName, const tooling::CompileCommand &Command, - PrecompiledPreamble const *Preamble, - const IncludeStructure &PreambleInclusions, StringRef Contents, - Position Pos, IntrusiveRefCntPtr VFS, + const PreambleData *Preamble, StringRef Contents, Position Pos, + IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, CodeCompleteOptions Opts, SpeculativeFuzzyFind *SpecFuzzyFind) { - return CodeCompleteFlow(FileName, PreambleInclusions, SpecFuzzyFind, Opts) + return CodeCompleteFlow(FileName, + Preamble ? Preamble->Includes : IncludeStructure(), + SpecFuzzyFind, Opts) .run({FileName, Command, Preamble, Contents, Pos, VFS, PCHs}); } SignatureHelp signatureHelp(PathRef FileName, const tooling::CompileCommand &Command, - PrecompiledPreamble const *Preamble, - StringRef Contents, Position Pos, + const PreambleData *Preamble, StringRef Contents, + Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, const SymbolIndex *Index) { Index: clangd/FS.h =================================================================== --- clangd/FS.h +++ clangd/FS.h @@ -0,0 +1,65 @@ +//===--- FS.h - File system related utils ------------------------*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H + +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/ADT/Optional.h" + +namespace clang { +namespace clangd { + +/// Records status information for files open()ed or stat()ed during preamble +/// build, so we can avoid stat()s on the underlying FS when reusing the +/// preamble. For example, code completion can re-stat files when getting FileID +/// for source locations stored in preamble (e.g. checking whether a location is +/// in the main file). +/// +/// The cache is keyed by absolute path of file name in cached status, as this +/// is what preamble stores. +/// +/// The cache is not thread-safe when updates happen, so the use pattern should +/// be: +/// - One FS writes to the cache from one thread (or several but strictly +/// sequenced), e.g. when building preamble. +/// - Sequence point (no writes after this point, no reads before). +/// - Several FSs can read from the cache, e.g. code completions. +/// +/// Note that the cache is only valid when reusing preamble. +class PreambleFileStatusCache { +public: + void update(const vfs::FileSystem &FS, vfs::Status S); + /// \p Path is a path stored in preamble. + llvm::Optional lookup(llvm::StringRef Path) const; + + /// Returns a VFS that collects file status. + /// Only cache stats for files that exist because + /// 1) we only care about existing files when reusing preamble, unlike + /// building preamble. + /// 2) we use the file name in the Status as the cache key. + /// + /// Note that the returned VFS should not outlive the cache. + IntrusiveRefCntPtr + getProducingFS(IntrusiveRefCntPtr FS); + + /// Returns a VFS that uses the cache collected. + /// + /// Note that the returned VFS should not outlive the cache. + IntrusiveRefCntPtr + getConsumingFS(IntrusiveRefCntPtr FS) const; + +private: + llvm::StringMap StatCache; +}; + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H Index: clangd/FS.cpp =================================================================== --- clangd/FS.cpp +++ clangd/FS.cpp @@ -0,0 +1,92 @@ +//===--- FS.cpp - File system related utils ----------------------*- C++-*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "FS.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/ADT/None.h" + +namespace clang { +namespace clangd { + +void PreambleFileStatusCache::update(const vfs::FileSystem &FS, vfs::Status S) { + SmallString<32> PathStore(S.getName()); + if (auto Err = FS.makeAbsolute(PathStore)) + return; + // Stores the latest status in cache as it can change in a preamble build. + StatCache.insert({PathStore, std::move(S)}); +} + +llvm::Optional +PreambleFileStatusCache::lookup(llvm::StringRef File) const { + auto I = StatCache.find(File); + if (I != StatCache.end()) + return I->getValue(); + return llvm::None; +} + +IntrusiveRefCntPtr PreambleFileStatusCache::getProducingFS( + IntrusiveRefCntPtr FS) { + // This invalidates old status in cache if files are re-`open()`ed or + // re-`stat()`ed in case file status has changed during preamble build. + class CollectFS : public vfs::ProxyFileSystem { + public: + CollectFS(IntrusiveRefCntPtr FS, + PreambleFileStatusCache &StatCache) + : ProxyFileSystem(std::move(FS)), StatCache(StatCache) {} + + llvm::ErrorOr> + openFileForRead(const Twine &Path) override { + auto File = getUnderlyingFS().openFileForRead(Path); + if (!File || !*File) + return File; + // Eagerly stat opened file, as the followup `status` call on the file + // doesn't necessarily go through this FS. This puts some extra work on + // preamble build, but it should be worth it as preamble can be reused + // 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)); + return File; + } + + llvm::ErrorOr status(const Twine &Path) override { + auto S = getUnderlyingFS().status(Path); + if (S) + StatCache.update(getUnderlyingFS(), *S); + return S; + } + + private: + PreambleFileStatusCache &StatCache; + }; + return IntrusiveRefCntPtr(new CollectFS(std::move(FS), *this)); +} + +IntrusiveRefCntPtr PreambleFileStatusCache::getConsumingFS( + IntrusiveRefCntPtr FS) const { + class CacheVFS : public vfs::ProxyFileSystem { + public: + CacheVFS(IntrusiveRefCntPtr FS, + const PreambleFileStatusCache &StatCache) + : ProxyFileSystem(std::move(FS)), StatCache(StatCache) {} + + llvm::ErrorOr status(const Twine &Path) override { + if (auto S = StatCache.lookup(Path.str())) + return *S; + return getUnderlyingFS().status(Path); + } + + private: + const PreambleFileStatusCache &StatCache; + }; + return IntrusiveRefCntPtr(new CacheVFS(std::move(FS), *this)); +} + +} // namespace clangd +} // namespace clang Index: unittests/clangd/CMakeLists.txt =================================================================== --- unittests/clangd/CMakeLists.txt +++ unittests/clangd/CMakeLists.txt @@ -21,6 +21,7 @@ FileDistanceTests.cpp FileIndexTests.cpp FindSymbolsTests.cpp + FSTests.cpp FuzzyMatchTests.cpp GlobalCompilationDatabaseTests.cpp HeadersTests.cpp Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -963,6 +963,71 @@ Field(&CodeCompletion::Name, "baz"))); } +// Check that running code completion doesn't stat() a bunch of files from the +// preamble again. (They should be using the preamble's stat-cache) +TEST(ClangdTests, PreambleVFSStatCache) { + class ListenStatsFSProvider : public FileSystemProvider { + public: + ListenStatsFSProvider(llvm::StringMap &CountStats) + : CountStats(CountStats) {} + + IntrusiveRefCntPtr getFileSystem() override { + class ListenStatVFS : public vfs::ProxyFileSystem { + public: + ListenStatVFS(IntrusiveRefCntPtr FS, + llvm::StringMap &CountStats) + : ProxyFileSystem(std::move(FS)), CountStats(CountStats) {} + + llvm::ErrorOr> + openFileForRead(const Twine &Path) override { + ++CountStats[llvm::sys::path::filename(Path.str())]; + return ProxyFileSystem::openFileForRead(Path); + } + llvm::ErrorOr status(const Twine &Path) override { + ++CountStats[llvm::sys::path::filename(Path.str())]; + return ProxyFileSystem::status(Path); + } + + private: + llvm::StringMap &CountStats; + }; + + return IntrusiveRefCntPtr( + new ListenStatVFS(buildTestFS(Files), CountStats)); + } + + // If relative paths are used, they are resolved with testPath(). + llvm::StringMap Files; + llvm::StringMap &CountStats; + }; + + llvm::StringMap CountStats; + ListenStatsFSProvider FS(CountStats); + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + auto SourcePath = testPath("foo.cpp"); + auto HeaderPath = testPath("foo.h"); + FS.Files[HeaderPath] = "struct TestSym {};"; + Annotations Code(R"cpp( + #include "foo.h" + + int main() { + TestSy^ + })cpp"); + + runAddDocument(Server, SourcePath, Code.code()); + + EXPECT_EQ(CountStats["foo.h"], 1u); + auto Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(), + clangd::CodeCompleteOptions())) + .Completions; + EXPECT_EQ(CountStats["foo.h"], 1u); + EXPECT_THAT(Completions, + ElementsAre(Field(&CodeCompletion::Name, "TestSym"))); +} + } // namespace } // namespace clangd } // namespace clang Index: unittests/clangd/FSTests.cpp =================================================================== --- unittests/clangd/FSTests.cpp +++ unittests/clangd/FSTests.cpp @@ -0,0 +1,46 @@ +//===-- FSTests.cpp - File system related tests -----------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "FS.h" +#include "TestFS.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +TEST(FSTests, PreambleStatusCache) { + llvm::StringMap Files; + Files["x"] = ""; + Files["y"] = ""; + auto FS = buildTestFS(Files); + FS->setCurrentWorkingDirectory(testRoot()); + + PreambleFileStatusCache StatCache; + auto ProduceFS = StatCache.getProducingFS(FS); + EXPECT_TRUE(ProduceFS->openFileForRead("x")); + EXPECT_TRUE(ProduceFS->status("y")); + + EXPECT_TRUE(StatCache.lookup(testPath("x")).hasValue()); + EXPECT_TRUE(StatCache.lookup(testPath("y")).hasValue()); + + vfs::Status S("fake", llvm::sys::fs::UniqueID(0, 0), + std::chrono::system_clock::now(), 0, 0, 1024, + llvm::sys::fs::file_type::regular_file, llvm::sys::fs::all_all); + StatCache.update(*FS, S); + auto ConsumeFS = StatCache.getConsumingFS(FS); + auto Cached = ConsumeFS->status(testPath("fake")); + EXPECT_TRUE(Cached); + EXPECT_EQ(Cached->getName(), S.getName()); +} + +} // namespace +} // namespace clangd +} // namespace clang Index: unittests/clangd/TestFS.cpp =================================================================== --- unittests/clangd/TestFS.cpp +++ unittests/clangd/TestFS.cpp @@ -23,6 +23,7 @@ llvm::StringMap const &Timestamps) { IntrusiveRefCntPtr MemFS( new vfs::InMemoryFileSystem); + MemFS->setCurrentWorkingDirectory(testRoot()); for (auto &FileAndContents : Files) { StringRef File = FileAndContents.first(); MemFS->addFile(