ioeric created this revision.
ioeric added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
ioeric updated this revision to Diff 166667.
ioeric added a comment.
- update comment
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.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52419
Files:
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/CodeComplete.h
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -963,6 +963,89 @@
Field(&CodeCompletion::Name, "baz")));
}
+TEST(ClangdTests, PreambleVFSStatCache) {
+ class ListenStatsFSProvider : public FileSystemProvider {
+ public:
+ ListenStatsFSProvider(llvm::StringMap<unsigned> &CountStats)
+ : CountStats(CountStats) {}
+
+ IntrusiveRefCntPtr<vfs::FileSystem> getFileSystem() override {
+ class ListenStatVFS : public vfs::FileSystem {
+ public:
+ ListenStatVFS(IntrusiveRefCntPtr<vfs::FileSystem> FS,
+ llvm::StringMap<unsigned> &CountStats)
+ : FS(std::move(FS)), CountStats(CountStats) {}
+
+ vfs::directory_iterator dir_begin(const Twine &Dir,
+ std::error_code &EC) override {
+ return FS->dir_begin(Dir, EC);
+ }
+ std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+ return FS->setCurrentWorkingDirectory(Path);
+ }
+ llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+ return FS->getCurrentWorkingDirectory();
+ }
+ std::error_code
+ getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output) const override {
+ return FS->getRealPath(Path, Output);
+ }
+
+ llvm::ErrorOr<std::unique_ptr<vfs::File>>
+ openFileForRead(const Twine &Path) override {
+ return FS->openFileForRead(Path);
+ }
+
+ llvm::ErrorOr<vfs::Status> status(const Twine &Path) override {
+ auto I =
+ CountStats.try_emplace(llvm::sys::path::filename(Path.str()), 1);
+ if (!I.second)
+ I.first->second += 1;
+ return FS->status(Path);
+ }
+
+ private:
+ IntrusiveRefCntPtr<vfs::FileSystem> FS;
+ 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());
+
+ unsigned Before = CountStats["foo.h"];
+ auto Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+ clangd::CodeCompleteOptions()))
+ .Completions;
+ EXPECT_EQ(CountStats["foo.h"], Before);
+ EXPECT_THAT(Completions,
+ ElementsAre(Field(&CodeCompletion::Name, "TestSym")));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.h
===================================================================
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -20,6 +20,7 @@
#include "Logger.h"
#include "Path.h"
#include "Protocol.h"
+#include "TUScheduler.h"
#include "index/Index.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Sema/CodeCompleteConsumer.h"
@@ -212,11 +213,8 @@
/// 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,
- StringRef Contents, Position Pos,
+CodeCompleteResult codeComplete(PathRef FileName, Position Pos,
+ const InputsAndPreamble &Input,
IntrusiveRefCntPtr<vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHs,
CodeCompleteOptions Opts,
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -1000,7 +1000,8 @@
struct SemaCompleteInput {
PathRef FileName;
const tooling::CompileCommand &Command;
- PrecompiledPreamble const *Preamble;
+ const PrecompiledPreamble *Preamble;
+ const PreambleFileStatusCache *PreambleStatCache;
StringRef Contents;
Position Pos;
IntrusiveRefCntPtr<vfs::FileSystem> VFS;
@@ -1024,12 +1025,16 @@
// working dirs.
}
+ IntrusiveRefCntPtr<vfs::FileSystem> VFS = Input.VFS;
+ if (Input.PreambleStatCache)
+ VFS = createVFSWithPreambleStatCache(std::move(VFS),
+ *Input.PreambleStatCache);
IgnoreDiagnostics DummyDiagsConsumer;
auto CI = createInvocationFromCommandLine(
ArgStrs,
CompilerInstance::createDiagnostics(new DiagnosticOptions,
&DummyDiagsConsumer, false),
- Input.VFS);
+ VFS);
if (!CI) {
elog("Couldn't create CompilerInvocation");
return false;
@@ -1069,7 +1074,7 @@
// 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(ContentsBuffer), std::move(Input.PCHs), std::move(VFS),
DummyDiagsConsumer);
Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
Clang->setCodeCompletionConsumer(Consumer.release());
@@ -1586,15 +1591,20 @@
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,
- std::shared_ptr<PCHContainerOperations> PCHs,
- CodeCompleteOptions Opts, SpeculativeFuzzyFind *SpecFuzzyFind) {
- return CodeCompleteFlow(FileName, PreambleInclusions, SpecFuzzyFind, Opts)
- .run({FileName, Command, Preamble, Contents, Pos, VFS, PCHs});
+CodeCompleteResult codeComplete(PathRef FileName, Position Pos,
+ const InputsAndPreamble &Input,
+ IntrusiveRefCntPtr<vfs::FileSystem> VFS,
+ std::shared_ptr<PCHContainerOperations> PCHs,
+ CodeCompleteOptions Opts,
+ SpeculativeFuzzyFind *SpecFuzzyFind) {
+ return CodeCompleteFlow(FileName,
+ Input.Preamble ? Input.Preamble->Includes
+ : IncludeStructure(),
+ SpecFuzzyFind, Opts)
+ .run({FileName, Input.Command,
+ Input.Preamble ? &Input.Preamble->Preamble : nullptr,
+ Input.Preamble ? &Input.Preamble->StatCache : nullptr,
+ Input.Contents, Pos, VFS, PCHs});
}
SignatureHelp signatureHelp(PathRef FileName,
@@ -1614,8 +1624,8 @@
semaCodeComplete(
llvm::make_unique<SignatureHelpCollector>(Options, Index, Result),
Options,
- {FileName, Command, Preamble, Contents, Pos, std::move(VFS),
- std::move(PCHs)});
+ {FileName, Command, Preamble, /*PreambleStatCache=*/nullptr, Contents,
+ Pos, std::move(VFS), std::move(PCHs)});
return Result;
}
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -15,12 +15,16 @@
#include "Headers.h"
#include "Path.h"
#include "Protocol.h"
+#include "clang/Basic/VirtualFileSystem.h"
#include "clang/Frontend/FrontendAction.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Serialization/ASTBitCodes.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
#include <memory>
#include <string>
#include <vector>
@@ -42,17 +46,32 @@
namespace clangd {
+/// Cached `stat()` calls during preamble build. The map keys are absolute paths
+/// of the file path in the cached status, which are the paths stored in
+/// preamble.
+using PreambleFileStatusCache = llvm::StringMap<llvm::ErrorOr<vfs::Status>>;
+
+/// Wraps \p FS with the \p StatCache. This should only be used when preamble
+/// is reused (e.g. code completion) and the cached status is valid.
+IntrusiveRefCntPtr<vfs::FileSystem>
+createVFSWithPreambleStatCache(IntrusiveRefCntPtr<vfs::FileSystem> FS,
+ const PreambleFileStatusCache &StatCache);
+
// Stores Preamble and associated data.
struct PreambleData {
PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags,
- IncludeStructure Includes);
+ IncludeStructure Includes, 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 `stat()` results when building preamble. This can be used to
+ // avoid re-`stat`s header files when preamble is re-used (e.g. in code
+ // completion). Note that the cache is only valid if preamble is reused.
+ const PreambleFileStatusCache 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
@@ -15,6 +15,7 @@
#include "Trace.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -29,6 +30,7 @@
#include "clang/Serialization/ASTWriter.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/raw_ostream.h"
@@ -114,6 +116,59 @@
SourceManager *SourceMgr = nullptr;
};
+/// Collect and cache all file status from the underlying file system.
+class CollectStatCacheVFS : public vfs::FileSystem {
+public:
+ CollectStatCacheVFS(IntrusiveRefCntPtr<vfs::FileSystem> FS,
+ PreambleFileStatusCache &StatCache)
+ : FS(std::move(FS)), StatCache(StatCache) {}
+
+ vfs::directory_iterator dir_begin(const Twine &Dir,
+ std::error_code &EC) override {
+ return FS->dir_begin(Dir, EC);
+ }
+ std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+ return FS->setCurrentWorkingDirectory(Path);
+ }
+ llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+ return FS->getCurrentWorkingDirectory();
+ }
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output) const override {
+ return FS->getRealPath(Path, Output);
+ }
+
+ llvm::ErrorOr<std::unique_ptr<vfs::File>>
+ openFileForRead(const Twine &Path) override {
+ auto File = FS->openFileForRead(Path);
+ if (!File || !*File)
+ return File;
+ // Only cache stats for files that exist because, unlike building preamble,
+ // we only care about existing files when reusing preamble.
+ if (auto S = File->get()->status())
+ cacheStatus(std::move(*S));
+ return File;
+ }
+
+ llvm::ErrorOr<vfs::Status> status(const Twine &Path) override {
+ auto S = FS->status(Path);
+ if (S)
+ cacheStatus(*S);
+ return S;
+ }
+
+private:
+ void cacheStatus(vfs::Status S) {
+ SmallString<32> PathStore(S.getName());
+ if (auto Err = makeAbsolute(PathStore))
+ return;
+ // Stores the latest status in cache as it can change in a preamble build.
+ StatCache.insert({PathStore, std::move(S)});
+ }
+ IntrusiveRefCntPtr<vfs::FileSystem> FS;
+ PreambleFileStatusCache &StatCache;
+};
+
} // namespace
void clangd::dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
@@ -246,9 +301,51 @@
}
PreambleData::PreambleData(PrecompiledPreamble Preamble,
- std::vector<Diag> Diags, IncludeStructure Includes)
+ std::vector<Diag> Diags, IncludeStructure Includes,
+ PreambleFileStatusCache StatCache)
: Preamble(std::move(Preamble)), Diags(std::move(Diags)),
- Includes(std::move(Includes)) {}
+ Includes(std::move(Includes)), StatCache(std::move(StatCache)) {}
+
+IntrusiveRefCntPtr<vfs::FileSystem> clangd::createVFSWithPreambleStatCache(
+ IntrusiveRefCntPtr<vfs::FileSystem> FS,
+ const PreambleFileStatusCache &StatCache) {
+ class CacheVFS : public vfs::FileSystem {
+ public:
+ CacheVFS(IntrusiveRefCntPtr<vfs::FileSystem> FS,
+ const PreambleFileStatusCache &StatCache)
+ : FS(std::move(FS)), StatCache(StatCache) {}
+
+ vfs::directory_iterator dir_begin(const Twine &Dir,
+ std::error_code &EC) override {
+ return FS->dir_begin(Dir, EC);
+ }
+ std::error_code setCurrentWorkingDirectory(const Twine &Path) override {
+ return FS->setCurrentWorkingDirectory(Path);
+ }
+ llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+ return FS->getCurrentWorkingDirectory();
+ }
+
+ llvm::ErrorOr<std::unique_ptr<vfs::File>>
+ openFileForRead(const Twine &Path) override {
+ return FS->openFileForRead(Path);
+ }
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl<char> &Output) const override {
+ return FS->getRealPath(Path, Output);
+ }
+
+ llvm::ErrorOr<vfs::Status> status(const Twine &Path) override {
+ auto I = StatCache.find(Path.str());
+ return (I != StatCache.end()) ? I->getValue() : FS->status(Path);
+ }
+
+ private:
+ IntrusiveRefCntPtr<vfs::FileSystem> FS;
+ const PreambleFileStatusCache &StatCache;
+ };
+ return IntrusiveRefCntPtr<CacheVFS>(new CacheVFS(std::move(FS), StatCache));
+}
ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
@@ -334,8 +431,12 @@
// We proceed anyway, our lit-tests rely on results for non-existing working
// dirs.
}
+
+ PreambleFileStatusCache StatCache;
+ llvm::IntrusiveRefCntPtr<vfs::FileSystem> CacheVFS(
+ new CollectStatCacheVFS(Inputs.FS, StatCache));
auto BuiltPreamble = PrecompiledPreamble::Build(
- CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Inputs.FS, PCHs,
+ CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, CacheVFS, PCHs,
StoreInMemory, SerializedDeclsCollector);
// When building the AST for the main file, we do want the function
@@ -347,7 +448,7 @@
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;
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -181,8 +181,6 @@
if (isCancelled())
return CB(llvm::make_error<CancelledError>());
- auto PreambleData = IP->Preamble;
-
llvm::Optional<SpeculativeFuzzyFind> SpecFuzzyFind;
if (CodeCompleteOpts.Index && CodeCompleteOpts.SpeculativeIndexRequest) {
SpecFuzzyFind.emplace();
@@ -195,9 +193,7 @@
// 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,
+ File, Pos, *IP, FS, PCHs, CodeCompleteOpts,
SpecFuzzyFind ? SpecFuzzyFind.getPointer() : nullptr);
{
clang::clangd::trace::Span Tracer("Completion results callback");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
