It didn't seem to work. I have temporarily disabled the test for windows (r343637).
On Tue, Oct 2, 2018 at 10:05 PM Eric Liu <ioe...@google.com> wrote: > Hi Douglas, > > Thanks for letting me know! The test seemed to be too strict for windows. > r343623 is an attempt to fix. If it still doesn't work, feel free to revert > the commits, and I'll re-investigate on Thursday. > > - Eric > > On Tue, Oct 2, 2018 at 9:25 PM <douglas.y...@sony.com> wrote: > >> Hi Eric, >> >> One of the tests you added in this commit is causing a failure on the PS4 >> Windows bot: >> >> >> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20347/steps/test/logs/stdio >> : >> >> FAIL: Extra Tools Unit Tests :: >> clangd/./ClangdTests.exe/ClangdTests.PreambleVFSStatCache (87 of 344) >> ******************** TEST 'Extra Tools Unit Tests :: >> clangd/./ClangdTests.exe/ClangdTests.PreambleVFSStatCache' FAILED >> ******************** >> Note: Google Test filter = ClangdTests.PreambleVFSStatCache >> >> [==========] Running 1 test from 1 test case. >> >> [----------] Global test environment set-up. >> >> [----------] 1 test from ClangdTests >> >> [ RUN ] ClangdTests.PreambleVFSStatCache >> >> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\tools\extra\unittests\clangd\ClangdTests.cpp(1026): >> error: Expected: CountStats["foo.h"] >> >> Which is: 2 >> >> To be equal to: 1u >> >> Which is: 1 >> >> [ FAILED ] ClangdTests.PreambleVFSStatCache (441 ms) >> >> [----------] 1 test from ClangdTests (441 ms total) >> >> >> >> [----------] Global test environment tear-down >> >> [==========] 1 test from 1 test case ran. (441 ms total) >> >> [ PASSED ] 0 tests. >> >> [ FAILED ] 1 test, listed below: >> >> [ FAILED ] ClangdTests.PreambleVFSStatCache >> >> >> >> 1 FAILED TEST >> >> Updating file C:\clangd-test\foo.cpp with command [C:\clangd-test] clang >> -ffreestanding C:\clangd-test\foo.cpp >> -resource-dir=C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\tools\clang\tools\extra\unittests\clangd\..\lib\clang\8.0.0 >> >> Preamble for file C:\clangd-test\foo.cpp cannot be reused. Attempting to >> rebuild it. >> >> Built preamble of size 185492 for file C:\clangd-test\foo.cpp >> >> Code complete: sema context Statement, query scopes [] (AnyScope=false) >> >> Code complete: 1 results from Sema, 0 from Index, 0 matched, 1 returned. >> >> ******************** >> >> Can you please take a look? >> >> Douglas Yung >> >> > -----Original Message----- >> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf >> > Of Eric Liu via cfe-commits >> > Sent: Tuesday, October 02, 2018 3:44 >> > To: cfe-commits@lists.llvm.org >> > Subject: [clang-tools-extra] r343576 - [clangd] Cache FS stat() calls >> > when building preamble. >> > >> > Author: ioeric >> > Date: Tue Oct 2 03:43:55 2018 >> > New Revision: 343576 >> > >> > URL: http://llvm.org/viewvc/llvm-project?rev=343576&view=rev >> > Log: >> > [clangd] Cache FS stat() calls when building preamble. >> > >> > 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. >> > >> > Reviewers: sammccall, ilya-biryukov >> > >> > Reviewed By: sammccall >> > >> > Subscribers: mgorny, MaskRay, jkorous, arphaman, kadircet, cfe-commits >> > >> > Differential Revision: https://reviews.llvm.org/D52419 >> > >> > Added: >> > clang-tools-extra/trunk/clangd/FS.cpp >> > clang-tools-extra/trunk/clangd/FS.h >> > clang-tools-extra/trunk/unittests/clangd/FSTests.cpp >> > Modified: >> > clang-tools-extra/trunk/clangd/CMakeLists.txt >> > clang-tools-extra/trunk/clangd/ClangdServer.cpp >> > clang-tools-extra/trunk/clangd/ClangdUnit.cpp >> > clang-tools-extra/trunk/clangd/ClangdUnit.h >> > clang-tools-extra/trunk/clangd/CodeComplete.cpp >> > clang-tools-extra/trunk/clangd/CodeComplete.h >> > clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt >> > clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp >> > clang-tools-extra/trunk/unittests/clangd/TestFS.cpp >> > >> > Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/CMakeLists.txt?rev=343576&r1=343575&r2=343576&view=d >> > iff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) >> > +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Oct 2 03:43:55 >> > 2018 >> > @@ -21,6 +21,7 @@ add_clang_library(clangDaemon >> > DraftStore.cpp >> > FindSymbols.cpp >> > FileDistance.cpp >> > + FS.cpp >> > FuzzyMatch.cpp >> > GlobalCompilationDatabase.cpp >> > Headers.cpp >> > >> > Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/ClangdServer.cpp?rev=343576&r1=343575&r2=343576&view >> > =diff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) >> > +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Oct 2 03:43:55 >> > 2018 >> > @@ -181,8 +181,6 @@ void ClangdServer::codeComplete(PathRef >> > if (isCancelled()) >> > return CB(llvm::make_error<CancelledError>()); >> > >> > - auto PreambleData = IP->Preamble; >> > - >> > llvm::Optional<SpeculativeFuzzyFind> SpecFuzzyFind; >> > if (CodeCompleteOpts.Index && >> > CodeCompleteOpts.SpeculativeIndexRequest) { >> > SpecFuzzyFind.emplace(); >> > @@ -195,10 +193,8 @@ void ClangdServer::codeComplete(PathRef >> > // 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 @@ void ClangdServer::signatureHelp(PathRef >> > 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. >> > >> > Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/ClangdUnit.cpp?rev=343576&r1=343575&r2=343576&view=d >> > iff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) >> > +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Oct 2 03:43:55 >> > 2018 >> > @@ -246,9 +246,10 @@ const IncludeStructure &ParsedAST::getIn >> > } >> > >> > 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, >> > @@ -334,9 +335,12 @@ std::shared_ptr<const PreambleData> clan >> > // 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. >> > @@ -347,7 +351,7 @@ std::shared_ptr<const PreambleData> clan >> > 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; >> > @@ -361,15 +365,19 @@ llvm::Optional<ParsedAST> clangd::buildA >> > 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, >> > >> > Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/ClangdUnit.h?rev=343576&r1=343575&r2=343576&view=dif >> > f >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/ClangdUnit.h (original) >> > +++ clang-tools-extra/trunk/clangd/ClangdUnit.h Tue Oct 2 03:43:55 >> > 2018 >> > @@ -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 @@ 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; >> > @@ -53,6 +55,9 @@ struct PreambleData { >> > // 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. >> > >> > Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/CodeComplete.cpp?rev=343576&r1=343575&r2=343576&view >> > =diff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) >> > +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Oct 2 03:43:55 >> > 2018 >> > @@ -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 @@ private: >> > struct SemaCompleteInput { >> > PathRef FileName; >> > const tooling::CompileCommand &Command; >> > - PrecompiledPreamble const *Preamble; >> > + const PreambleData *Preamble; >> > StringRef Contents; >> > Position Pos; >> > IntrusiveRefCntPtr<vfs::FileSystem> VFS; >> > @@ -1010,12 +1011,15 @@ bool semaCodeComplete(std::unique_ptr<Co >> > // 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; >> > @@ -1054,8 +1058,10 @@ bool semaCodeComplete(std::unique_ptr<Co >> > // 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 @@ speculateCompletionFilter(llvm::StringRe >> > >> > 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) { >> > >> > Modified: clang-tools-extra/trunk/clangd/CodeComplete.h >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/CodeComplete.h?rev=343576&r1=343575&r2=343576&view=d >> > iff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/CodeComplete.h (original) >> > +++ clang-tools-extra/trunk/clangd/CodeComplete.h Tue Oct 2 03:43:55 >> > 2018 >> > @@ -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 @@ struct SpeculativeFuzzyFind { >> > /// 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, >> > @@ -232,8 +232,8 @@ CodeCompleteResult codeComplete(PathRef >> > /// 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); >> > >> > Added: clang-tools-extra/trunk/clangd/FS.cpp >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/FS.cpp?rev=343576&view=auto >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/FS.cpp (added) >> > +++ clang-tools-extra/trunk/clangd/FS.cpp Tue Oct 2 03:43:55 2018 >> > @@ -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<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 >> > >> > Added: clang-tools-extra/trunk/clangd/FS.h >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/clangd/FS.h?rev=343576&view=auto >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/clangd/FS.h (added) >> > +++ clang-tools-extra/trunk/clangd/FS.h Tue Oct 2 03:43:55 2018 >> > @@ -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<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 >> > >> > Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/unittests/clangd/CMakeLists.txt?rev=343576&r1=343575&r2=343 >> > 576&view=diff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original) >> > +++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Tue Oct 2 >> > 03:43:55 2018 >> > @@ -21,6 +21,7 @@ add_extra_unittest(ClangdTests >> > FileDistanceTests.cpp >> > FileIndexTests.cpp >> > FindSymbolsTests.cpp >> > + FSTests.cpp >> > FuzzyMatchTests.cpp >> > GlobalCompilationDatabaseTests.cpp >> > HeadersTests.cpp >> > >> > Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/unittests/clangd/ClangdTests.cpp?rev=343576&r1=343575&r2=34 >> > 3576&view=diff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original) >> > +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Tue Oct 2 >> > 03:43:55 2018 >> > @@ -963,6 +963,71 @@ TEST_F(ClangdVFSTest, ChangedHeaderFromI >> > 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 >> > >> > Added: clang-tools-extra/trunk/unittests/clangd/FSTests.cpp >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/unittests/clangd/FSTests.cpp?rev=343576&view=auto >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/unittests/clangd/FSTests.cpp (added) >> > +++ clang-tools-extra/trunk/unittests/clangd/FSTests.cpp Tue Oct 2 >> > 03:43:55 2018 >> > @@ -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<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 >> > >> > Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp >> > URL: http://llvm.org/viewvc/llvm-project/clang-tools- >> > extra/trunk/unittests/clangd/TestFS.cpp?rev=343576&r1=343575&r2=343576& >> > view=diff >> > ======================================================================= >> > ======= >> > --- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original) >> > +++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Tue Oct 2 >> > 03:43:55 2018 >> > @@ -23,6 +23,7 @@ buildTestFS(llvm::StringMap<std::string> >> > 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( >> > >> > >> > _______________________________________________ >> > cfe-commits mailing list >> > cfe-commits@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
