[clangd] Cache FS stat() calls when building preamble.

[ioeric]

Summary

The file stats can be reused when preamble is reused (e.g. code
completion). It's safe to assume that cached status is not outdated as we
assume preamble files to remain unchanged.

On real file system, this made code completion ~20% faster on a measured file
(with big preamble). The preamble build time doesn't change much.

[ioeric]

• update comment

[ioeric]

clangd/ClangdUnit.cpp

119 ↗

(On Diff #166667)

Would it make sense to add a clang::vfs::ProxyFS that proxies calls to underlying FS by default and allows to override some methods?

[ilya-biryukov]

clangd/ClangdUnit.cpp

119 ↗

(On Diff #166667)

This makes sense, we seem to have a recurring pattern of forwarding all methods by default in this commit.
+1 to the suggestion

128 ↗

(On Diff #166667)

One can obtain file stats from the returned iterator.
I believe this is exactly what will start happening after Sam's patch to list dirs when processing include dirs.

We should probably also wrap iterators that we return and cache stats from those.

149 ↗

(On Diff #166667)

If we cache the stats, we should probably cache the file contents too.
Not sure how much of an extra memory this will require.

Though maybe we could get away with returning the old stat, but new file contents. It would look as if the file was between calls to stat() and openForRead.

339 ↗

(On Diff #166667)

We store absolute paths, but Path can be relative here.

340 ↗

(On Diff #166667)

Maybe remove parens?
Comparisons associate properly with ternary op and it's common enough to not cause any confusion with the reader.

clangd/CodeComplete.cpp

1003 ↗

(On Diff #166667)

Maybe pass a single pointer to PreambleData here?

1028 ↗

(On Diff #166667)

It feels like we could apply this caching one level higher, on the TUScheduler level when creating the filesystem. It makes sense not only for code completion, but also for all other features, including rebuilds of ASTs that were washed out of the cache.
WDYT?

1627 ↗

(On Diff #166667)

signatureHelp could also benefit from the cached fs, maybe pass it too?
See the other comments about applying this caching one level higher (on TUScheduler level) too.

clangd/CodeComplete.h

217 ↗

(On Diff #166667)

We should not be depending TUScheduler.h, I suggest to pass PreambleDatainstead of PrecompiledPreamble instead.

Depending on TUScheduler.h our layering in clangd: low-level implementation of features should not depend on higher-level building blocks like TUScheduler.

[sammccall]

Nice win!

This is missing high-level documentation explaining what problem it's trying to solve and what the lifetime is.
I think pulling the factories out into a dedicated header would give you good space for that.

clangd/ClangdUnit.cpp

128 ↗

(On Diff #166667)

The returned iterator can't be used to obtain file status (since my patch).

149 ↗

(On Diff #166667)

I don't think I agree, what's the argument for caching both?

In the observed profile, stats are a problem but openFileForRead is not (in fact the files are never open).

This can't be a hard correctness requirement, because stat->open is racy anyway.

155 ↗

(On Diff #166667)

why are you only caching success?

166 ↗

(On Diff #166667)

this isn't threadsafe.

Note that adding a lock here isn't enough, as you may have multiple FS instances referring to the same cache. The lock needs to live alongside the cache storage, so the latter should probably be a class.

312 ↗

(On Diff #166667)

outline this implementation next to the other one, and give them parallel names?

statCacheProducingFS/statCacheConsumingFS?

The design is hard to follow without this parallelism explicit.

clangd/ClangdUnit.h

57 ↗

(On Diff #166667)

This doesn't belong in ClangdUnit.
(Potentially it could live in clang as a VFS util, but a FS.h or so here seems fine. Maybe merge with FSProvider.h)

clangd/CodeComplete.h

217 ↗

(On Diff #166667)

+1. We're maybe not doing the best job of header encapsulation here, but InputsAndPreamble is an implementation detail of ClangdServer.

[ioeric]

• Address review comments

[ioeric]

Thanks for the reviews!

clangd/ClangdUnit.cpp

155 ↗

(On Diff #166667)

I had a comment in openFileForRead:

// Only cache stats for files that exist because, unlike building preamble,
// we only care about existing files when reusing preamble.

Another reason is that we are using the file name in the Status as the cache key.

339 ↗

(On Diff #166667)

This is because preamble stores absolute paths. Path should be an path from preamble. Added documentation.

clangd/CodeComplete.cpp

1028 ↗

(On Diff #166667)

It sounds like a reasonable thing to do. I took a quick look, and it seemed that this would probably require some refactoring/redesign on TUScheduler - it doesn't know about the VFS?

I think it shouldn't be hard to do this case by case. For example, code completion and signature help will be covered in this patch. And it should be pretty easy to make AST rebuild use the cache as well?

clangd/CodeComplete.h

217 ↗

(On Diff #166667)

I thought InputsAndPreamble was a pretty natural fit here, but I guess I hadn't understand the abstractions well enough. Thanks for the explanation!

[sammccall]

Generally LG. A few things I'm unsure about.

clangd/CodeComplete.cpp

1028 ↗

(On Diff #166667)

It sounds like a reasonable thing to do. I took a quick look, and it seemed that this would probably require some refactoring/redesign on TUScheduler - it doesn't know about the VFS?

Right, this is a bit of a mess...
Currently there are features that access the FS "directly". This includes CodeComplete which runs its own compile action, as well as things like switchSourceHeader or format. These invoke FSProvider.
Then there are features that build an AST (which may need file accesses), and then may implicitly read files by querying the AST (the FS is attached to the FileManager or something). These use the FS obtained from the FSProvider in the relevant addDocument call.
There's no fundamental reason we can't have both (access FS directly and use the AST) in which case we'll be using both filesystems together...

In the near term, making the TUScheduler-managed AST rebuild use the cache stored in the preamble seems like a nice win. (As you alluded to I don't think this change is in TUScheduler itself, rather buildAST?)

clangd/FS.cpp

1 ↗

(On Diff #166951)

Can we have unit tests for these?
It should be pretty straightforward, as you can inspect/seed the cache state directly.

29 ↗

(On Diff #166951)

lock

48 ↗

(On Diff #166951)

I'm not sure I get this: AFAICT (at least on linux) the status is never available on a newly opened file, so this will always be a stat() call, so we're just doing the work eagerly and caching it for later. Isn't this just moving the work around?

I'm sure you've verified this is important somehow, but a comment explaining how would be much appreciated :-)

clangd/FS.h

20 ↗

(On Diff #166951)

can you give a little more colour on *why* code completion tends to stat a bunch of the same files over again?
(One might assume it's to validate the preamble, but it shouldn't be, because we don't do that for other reasons)

21 ↗

(On Diff #166951)

whene -> when

unittests/clangd/ClangdTests.cpp

966 ↗

(On Diff #166951)

it's not obvious what this test does (without the rest of the patch as context).
Maybe a high-level comment: 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)

980 ↗

(On Diff #166951)

just ++CountStats[...]?
life is too short :-)

984 ↗

(On Diff #166951)

or just return ProxyFileSystem::status(Path); which is *slightly* less coupled

1018 ↗

(On Diff #166951)

I think this would be much clearer if we asserted the actual count here. It should be 1, right? Do you think it'd be too brittle?

As it stands, it seems entirely possible that the test is not seeing any stats for whatever reason and is spuriously passing.

[ilya-biryukov]

clangd/ClangdUnit.cpp

339 ↗

(On Diff #166667)

It's true that preamble stores and checks absolute paths, however clang can later access the same file using a relative path.
Calling makeAbsolute here shouldn't hurt and would obviously make it work in both cases.

clangd/CodeComplete.cpp

1028 ↗

(On Diff #166667)

My idea would be to create and pass the updated cached FS into both buildAST and codeComplete higher up the call chain. This would allow localizing the code that creates caching VFSes in one file (TUScheduler.h or similar).

The advantage of receiving the patched-up FS in buildAST and codeComplete is that they don't have to care about this trick, making the implementation simpler.
The fewer functions across the codebase have to apply the trick the better, e.g. this would make sure we won't accidentally forget it to apply it when implementing a new feature.

[ioeric]

• address review comments

[ioeric]

clangd/ClangdUnit.cpp

339 ↗

(On Diff #166667)

It would "obviously work"... until you have symlinks and tricks to handle symlinks ;)

clangd/CodeComplete.cpp

1028 ↗

(On Diff #166667)

Added the cache support to buildAST as we thinks it's beneficial (haven't profiled this though).

Currently, the trick is applied in two places (semaCodeComplete in CodeComplete and buildAST in ClangdUnit), and I'm not sure how much this would grow in the near future. It's also unclear to me whether baking this into TUScheduler would be less work in the long run. I would suggest revisiting this when we have more evidence.

clangd/FS.cpp

29 ↗

(On Diff #166951)

After a second thought, I'm wondering if the mutex is necessary, if the cache is only updated during preamble build in a single thread. The cache would also remain the same in preamble reuses.

48 ↗

(On Diff #166951)

Files opened via openFileForRead() wouldn't usually trigger status() calls on the wrap FS. And most header files are opened instead of stated.

Added comment explaining this.

[sammccall]

clangd/FS.cpp

53 ↗

(On Diff #167124)

do you want to check if the file is already in the stat cache first?

29 ↗

(On Diff #166951)

Indeed if you have the following sequencing:

• one FS writes to the cache from one thread (or several but strictly sequenced)
• sequence point (no writes after this point, no reads before)
• several FSs read from the cache

then no lock is required, either for writer or reader.
The API doesn't particularly suggest this, so please explicitly document the threading model in the header.
(An API that would suggest it is to e.g. make the producing FS the top-level object, give it a &&-qualified method to retrieve the cache, and give the cache methods to retrieve the consuming FSes. But I think that's awkward here with the VFS interface)

48 ↗

(On Diff #166951)

Ah, OK.
The implementation comment is good, but this is significantly different from "caching stat calls" as described in the header files.

Maybe update the comment there: e.g. "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"

[ioeric]

• Address review comments
• address review comments
• address review comments

clangd/FS.cpp

53 ↗

(On Diff #167124)

This would always invalidate status in cache when file is reopened or restated in case file status has changed during preamble build. Added a comment.

29 ↗

(On Diff #166951)

(Stole some wording here. Thanks!)

[sammccall]

Sorry for dropping this.

clangd/ClangdUnit.h

58 ↗

(On Diff #167370)

This deserves a comment, even if it's a bit repetitive.

// Cache of FS operations performed when building the preamble.
// When reusing a preamble, this cache can be consumed to save IO.

[ioeric]

• add comment for StatCache in PreambleData

[ioeric]

• Rebase

clang-tools-extra/trunk/clangd/CMakeLists.txt

add_clang_library(clangDaemon
   CodeComplete.cpp
   CodeCompletionStrings.cpp
   Compiler.cpp
   Context.cpp
   Diagnostics.cpp
   DraftStore.cpp
   FindSymbols.cpp
   FileDistance.cpp
+  FS.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp
   JSONRPCDispatcher.cpp
   Logger.cpp
   Protocol.cpp
   ProtocolHandlers.cpp
   Quality.cpp

clang-tools-extra/trunk/clangd/ClangdServer.cpp

void ClangdServer::codeComplete(PathRef File, Position Pos,
   auto Task = [PCHs, Pos, FS, CodeCompleteOpts,
                this](Path File, Callback<CodeCompleteResult> CB,
                      llvm::Expected<InputsAndPreamble> IP) {
     if (!IP)
       return CB(IP.takeError());
     if (isCancelled())
       return CB(llvm::make_error<CancelledError>());
 
-    auto PreambleData = IP->Preamble;
-
     llvm::Optional<SpeculativeFuzzyFind> SpecFuzzyFind;
     if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) {
       SpecFuzzyFind.emplace();
       {
         std::lock_guard<std::mutex> Lock(CachedCompletionFuzzyFindRequestMutex);
         SpecFuzzyFind->CachedReq = CachedCompletionFuzzyFindRequestByFile[File];
       }
     }
 
     // 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));
     }
     if (SpecFuzzyFind && SpecFuzzyFind->NewReq.hasValue()) {
       std::lock_guard<std::mutex> Lock(CachedCompletionFuzzyFindRequestMutex);
       CachedCompletionFuzzyFindRequestByFile[File] =
           SpecFuzzyFind->NewReq.getValue();
void ClangdServer::signatureHelp(PathRef File, Position Pos,
   auto FS = FSProvider.getFileSystem();
   auto *Index = this->Index;
   auto Action = [Pos, FS, PCHs, Index](Path File, Callback<SignatureHelp> CB,
                                        llvm::Expected<InputsAndPreamble> IP) {
     if (!IP)
       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.
   // Signature help is often triggered after code completion. If the code
   // completion inserted a header to make the symbol available, then using
   // the old preamble would yield useless results.
   WorkScheduler.runWithPreamble("SignatureHelp", File, TUScheduler::Consistent,
                                 Bind(Action, File.str(), std::move(CB)));

clang-tools-extra/trunk/clangd/ClangdUnit.h

 //===--- ClangdUnit.h --------------------------------------------*- 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_CLANGDUNIT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H
 
 #include "Diagnostics.h"
+#include "FS.h"
 #include "Function.h"
 #include "Headers.h"
 #include "Path.h"
 #include "Protocol.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Serialization/ASTBitCodes.h"

 struct CompileCommand;
 }
 
 namespace clangd {
 
 // Stores Preamble and associated data.
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags,
-               IncludeStructure Includes);
+               IncludeStructure Includes,
+               std::unique_ptr<PreambleFileStatusCache> StatCache);
 
   tooling::CompileCommand CompileCommand;
   PrecompiledPreamble Preamble;
   std::vector<Diag> Diags;
   // 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<PreambleFileStatusCache> StatCache;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.
 struct ParseInputs {
   tooling::CompileCommand CompileCommand;
   IntrusiveRefCntPtr<vfs::FileSystem> FS;
   std::string Contents;
 };

clang-tools-extra/trunk/clangd/ClangdUnit.cpp

std::size_t ParsedAST::getUsedBytes() const {
   return Total;
 }
 
 const IncludeStructure &ParsedAST::getIncludeStructure() const {
   return Includes;
 }
 
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
-                           std::vector<Diag> Diags, IncludeStructure Includes)
+                           std::vector<Diag> Diags, IncludeStructure Includes,
+                           std::unique_ptr<PreambleFileStatusCache> 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<const PreambleData> Preamble,
                      std::unique_ptr<CompilerInstance> Clang,
                      std::unique_ptr<FrontendAction> Action,
                      std::vector<Decl *> LocalTopLevelDecls,
                      std::vector<Diag> Diags, IncludeStructure Includes)
     : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
       Action(std::move(Action)), Diags(std::move(Diags)),
std::shared_ptr<const PreambleData> clangd::buildPreamble(
   CI.getPreprocessorOpts().WriteCommentListToPCH = false;
 
   CppFilePreambleCallbacks SerializedDeclsCollector(FileName, PreambleCallback);
   if (Inputs.FS->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.
   }
+
+  auto StatCache = llvm::make_unique<PreambleFileStatusCache>();
   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.
   CI.getFrontendOpts().SkipFunctionBodies = false;
 
   if (BuiltPreamble) {
     vlog("Built preamble of size {0} for file {1}", BuiltPreamble->getSize(),
          FileName);
     return std::make_shared<PreambleData>(
         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;
   }
 }
 
 llvm::Optional<ParsedAST> clangd::buildAST(
     PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
     const ParseInputs &Inputs, std::shared_ptr<const PreambleData> Preamble,
     std::shared_ptr<PCHContainerOperations> PCHs) {
   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<CompilerInvocation>(*Invocation), Preamble,
-      llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), PCHs, Inputs.FS);
+  return ParsedAST::build(llvm::make_unique<CompilerInvocation>(*Invocation),
+                          Preamble,
+                          llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents),
+                          PCHs, std::move(VFS));
 }
 
 SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit,
                                                 const Position &Pos,
                                                 const FileID FID) {
   const ASTContext &AST = Unit.getASTContext();
   const SourceManager &SourceMgr = AST.getSourceManager();
   auto Offset = positionToOffset(SourceMgr.getBufferData(FID), Pos);

clang-tools-extra/trunk/clangd/CodeComplete.h

 // After "std::string S; S." we might suggest members of std::string.
 // Signature help describes the parameters of a function as you type them.
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "Protocol.h"
 #include "index/Index.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/CodeCompleteOptions.h"

 /// Get code completions at a specified \p Pos in \p FileName.
 /// If \p SpecFuzzyFind is set, a speculative and asynchronous fuzzy find index
 /// request (based on cached request) will be run before parsing sema. In case
 /// the speculative result is used by code completion (e.g. speculation failed),
 /// the speculative result is not consumed, and `SpecFuzzyFind` is only
 /// 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::FileSystem> VFS,
                                 std::shared_ptr<PCHContainerOperations> PCHs,
                                 CodeCompleteOptions Opts,
                                 SpeculativeFuzzyFind *SpecFuzzyFind = nullptr);
 
 /// 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::FileSystem> VFS,
                             std::shared_ptr<PCHContainerOperations> PCHs,
                             const SymbolIndex *Index);
 
 // For index-based completion, we only consider:
 //   * symbols in namespaces or translation unit scopes (e.g. no class
 //     members, no locals)
 //   * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")

clang-tools-extra/trunk/clangd/CodeComplete.cpp

 //
 // Signature help works in a similar way as code completion, but it is simpler:
 // it's purely AST-based, and there are few candidates.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CodeComplete.h"
 #include "AST.h"
+#include "ClangdUnit.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "FileDistance.h"
 #include "FuzzyMatch.h"
 #include "Headers.h"
 #include "Logger.h"
 #include "Quality.h"
private:
   std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator;
   CodeCompletionTUInfo CCTUInfo;
   const SymbolIndex *Index;
 }; // SignatureHelpCollector
 
 struct SemaCompleteInput {
   PathRef FileName;
   const tooling::CompileCommand &Command;
-  PrecompiledPreamble const *Preamble;
+  const PreambleData *Preamble;
   StringRef Contents;
   Position Pos;
   IntrusiveRefCntPtr<vfs::FileSystem> VFS;
   std::shared_ptr<PCHContainerOperations> PCHs;
 };
 
 // Invokes Sema code completion on a file.
 // If \p Includes is set, it will be updated based on the compiler invocation.
 bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
                       const clang::CodeCompleteOptions &Options,
                       const SemaCompleteInput &Input,
                       IncludeStructure *Includes = nullptr) {
   trace::Span Tracer("Sema completion");
   std::vector<const char *> ArgStrs;
   for (const auto &S : Input.Command.CommandLine)
     ArgStrs.push_back(S.c_str());
 
   if (Input.VFS->setCurrentWorkingDirectory(Input.Command.Directory)) {
     log("Couldn't set working directory");
     // We run parsing anyway, our lit-tests rely on results for non-existing
     // working dirs.
   }
 
+  IntrusiveRefCntPtr<vfs::FileSystem> 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;
   }
   auto &FrontendOpts = CI->getFrontendOpts();
   FrontendOpts.DisableFree = false;
   FrontendOpts.SkipFunctionBodies = true;
   CI->getLangOpts()->CommentOpts.ParseAllComments = true;
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
   // overriding the preamble will break sema completion. Fortunately we can just
   // skip all includes in this case; these completions are really simple.
   bool CompletingInPreamble =
       ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size >
       *Offset;
   // 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());
 
   SyntaxOnlyAction Action;
   if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) {
     log("BeginSourceFile() failed when running codeComplete for {0}",
         Input.FileName);
speculateCompletionFilter(llvm::StringRef Content, Position Pos) {
   }
   if (Len > 0)
     St++; // Shift to the first valid character.
   return Content.substr(St, Len);
 }
 
 CodeCompleteResult
 codeComplete(PathRef FileName, const tooling::CompileCommand &Command,
-             PrecompiledPreamble const *Preamble,
-             const IncludeStructure &PreambleInclusions, StringRef Contents,
-             Position Pos, IntrusiveRefCntPtr<vfs::FileSystem> VFS,
+             const PreambleData *Preamble, StringRef Contents, Position Pos,
+             IntrusiveRefCntPtr<vfs::FileSystem> VFS,
              std::shared_ptr<PCHContainerOperations> 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::FileSystem> VFS,
                             std::shared_ptr<PCHContainerOperations> PCHs,
                             const SymbolIndex *Index) {
   SignatureHelp Result;
   clang::CodeCompleteOptions Options;
   Options.IncludeGlobals = false;
   Options.IncludeMacros = false;
   Options.IncludeCodePatterns = false;

clang-tools-extra/trunk/clangd/FS.h

+//===--- 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<vfs::Status> 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<vfs::FileSystem>
+  getProducingFS(IntrusiveRefCntPtr<vfs::FileSystem> FS);
+
+  /// Returns a VFS that uses the cache collected.
+  ///
+  /// Note that the returned VFS should not outlive the cache.
+  IntrusiveRefCntPtr<vfs::FileSystem>
+  getConsumingFS(IntrusiveRefCntPtr<vfs::FileSystem> FS) const;
+
+private:
+  llvm::StringMap<vfs::Status> StatCache;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H

clang-tools-extra/trunk/clangd/FS.cpp

+//===--- 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<vfs::Status>
+PreambleFileStatusCache::lookup(llvm::StringRef File) const {
+  auto I = StatCache.find(File);
+  if (I != StatCache.end())
+    return I->getValue();
+  return llvm::None;
+}
+
+IntrusiveRefCntPtr<vfs::FileSystem> PreambleFileStatusCache::getProducingFS(
+    IntrusiveRefCntPtr<vfs::FileSystem> 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<vfs::FileSystem> FS,
+              PreambleFileStatusCache &StatCache)
+        : ProxyFileSystem(std::move(FS)), StatCache(StatCache) {}
+
+    llvm::ErrorOr<std::unique_ptr<vfs::File>>
+    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<vfs::Status> status(const Twine &Path) override {
+      auto S = getUnderlyingFS().status(Path);
+      if (S)
+        StatCache.update(getUnderlyingFS(), *S);
+      return S;
+    }
+
+  private:
+    PreambleFileStatusCache &StatCache;
+  };
+  return IntrusiveRefCntPtr<CollectFS>(new CollectFS(std::move(FS), *this));
+}
+
+IntrusiveRefCntPtr<vfs::FileSystem> PreambleFileStatusCache::getConsumingFS(
+    IntrusiveRefCntPtr<vfs::FileSystem> FS) const {
+  class CacheVFS : public vfs::ProxyFileSystem {
+  public:
+    CacheVFS(IntrusiveRefCntPtr<vfs::FileSystem> FS,
+             const PreambleFileStatusCache &StatCache)
+        : ProxyFileSystem(std::move(FS)), StatCache(StatCache) {}
+
+    llvm::ErrorOr<vfs::Status> status(const Twine &Path) override {
+      if (auto S = StatCache.lookup(Path.str()))
+        return *S;
+      return getUnderlyingFS().status(Path);
+    }
+
+  private:
+    const PreambleFileStatusCache &StatCache;
+  };
+  return IntrusiveRefCntPtr<CacheVFS>(new CacheVFS(std::move(FS), *this));
+}
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt

add_extra_unittest(ClangdTests
   CodeCompleteTests.cpp
   CodeCompletionStringsTests.cpp
   ContextTests.cpp
   DexTests.cpp
   DraftStoreTests.cpp
   FileDistanceTests.cpp
   FileIndexTests.cpp
   FindSymbolsTests.cpp
+  FSTests.cpp
   FuzzyMatchTests.cpp
   GlobalCompilationDatabaseTests.cpp
   HeadersTests.cpp
   IndexTests.cpp
   QualityTests.cpp
   RIFFTests.cpp
   SerializationTests.cpp
   SourceCodeTests.cpp

clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) {
   Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
                                          clangd::CodeCompleteOptions()))
                     .Completions;
   // We want to make sure we see the updated version.
   EXPECT_THAT(Completions, ElementsAre(Field(&CodeCompletion::Name, "bar"),
                                        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<unsigned> &CountStats)
+        : CountStats(CountStats) {}
+
+    IntrusiveRefCntPtr<vfs::FileSystem> getFileSystem() override {
+      class ListenStatVFS : public vfs::ProxyFileSystem {
+      public:
+        ListenStatVFS(IntrusiveRefCntPtr<vfs::FileSystem> FS,
+                      llvm::StringMap<unsigned> &CountStats)
+            : ProxyFileSystem(std::move(FS)), CountStats(CountStats) {}
+
+        llvm::ErrorOr<std::unique_ptr<vfs::File>>
+        openFileForRead(const Twine &Path) override {
+          ++CountStats[llvm::sys::path::filename(Path.str())];
+          return ProxyFileSystem::openFileForRead(Path);
+        }
+        llvm::ErrorOr<vfs::Status> status(const Twine &Path) override {
+          ++CountStats[llvm::sys::path::filename(Path.str())];
+          return ProxyFileSystem::status(Path);
+        }
+
+      private:
+        llvm::StringMap<unsigned> &CountStats;
+      };
+
+      return IntrusiveRefCntPtr<ListenStatVFS>(
+          new ListenStatVFS(buildTestFS(Files), CountStats));
+    }
+
+    // If relative paths are used, they are resolved with testPath().
+    llvm::StringMap<std::string> Files;
+    llvm::StringMap<unsigned> &CountStats;
+  };
+
+  llvm::StringMap<unsigned> 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

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

+//===-- 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<std::string> 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

clang-tools-extra/trunk/unittests/clangd/TestFS.cpp

 namespace clangd {
 using namespace llvm;
 
 IntrusiveRefCntPtr<vfs::FileSystem>
 buildTestFS(llvm::StringMap<std::string> const &Files,
             llvm::StringMap<time_t> const &Timestamps) {
   IntrusiveRefCntPtr<vfs::InMemoryFileSystem> MemFS(
       new vfs::InMemoryFileSystem);
+  MemFS->setCurrentWorkingDirectory(testRoot());
   for (auto &FileAndContents : Files) {
     StringRef File = FileAndContents.first();
     MemFS->addFile(
         File, Timestamps.lookup(File),
         MemoryBuffer::getMemBufferCopy(FileAndContents.second, File));
   }
   return MemFS;
 }