Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -70,6 +70,9 @@ /// If 0, all requests are processed on the calling thread. unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount(); + /// AST caching policy. The default is to keep up to 3 ASTs in memory. + ASTRetentionPolicy RetentionPolicy; + /// Cached preambles are potentially large. If false, store them on disk. bool StorePreamblesInMemory = true; Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -100,7 +100,7 @@ std::shared_ptr PP) { FileIdx->update(Path, &AST, std::move(PP)); } : PreambleParsedCallback(), - Opts.UpdateDebounce) { + Opts.UpdateDebounce, Opts.RetentionPolicy) { if (FileIdx && Opts.StaticIndex) { MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex); Index = MergedIndex.get(); Index: clang-tools-extra/trunk/clangd/ClangdUnit.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.h +++ clang-tools-extra/trunk/clangd/ClangdUnit.h @@ -47,6 +47,7 @@ PreambleData(PrecompiledPreamble Preamble, std::vector Diags, std::vector Inclusions); + tooling::CompileCommand CompileCommand; PrecompiledPreamble Preamble; std::vector Diags; // Processes like code completions and go-to-definitions will need #include @@ -128,50 +129,32 @@ using PreambleParsedCallback = std::function)>; -/// Manages resources, required by clangd. Allows to rebuild file with new -/// contents, and provides AST and Preamble for it. -class CppFile { -public: - CppFile(PathRef FileName, bool StorePreamblesInMemory, - std::shared_ptr PCHs, - PreambleParsedCallback PreambleCallback); - - /// Rebuild the AST and the preamble. - /// Returns a list of diagnostics or llvm::None, if an error occured. - llvm::Optional> rebuild(ParseInputs &&Inputs); - /// Returns the last built preamble. - const std::shared_ptr &getPreamble() const; - /// Returns the last built AST. - ParsedAST *getAST() const; - /// Returns an estimated size, in bytes, currently occupied by the AST and the - /// Preamble. - std::size_t getUsedBytes() const; - -private: - /// Build a new preamble for \p Inputs. If the current preamble can be reused, - /// it is returned instead. - /// This method is const to ensure we don't incidentally modify any fields. - std::shared_ptr - rebuildPreamble(CompilerInvocation &CI, - const tooling::CompileCommand &Command, - IntrusiveRefCntPtr FS, - llvm::MemoryBuffer &ContentsBuffer) const; - - const Path FileName; - const bool StorePreamblesInMemory; - - /// The last CompileCommand used to build AST and Preamble. - tooling::CompileCommand Command; - /// The last parsed AST. - llvm::Optional AST; - /// The last built Preamble. - std::shared_ptr Preamble; - /// Utility class required by clang - std::shared_ptr PCHs; - /// This is called after the file is parsed. This can be nullptr if there is - /// no callback. - PreambleParsedCallback PreambleCallback; -}; +/// Builds compiler invocation that could be used to build AST or preamble. +std::unique_ptr +buildCompilerInvocation(const ParseInputs &Inputs); + +/// Rebuild the preamble for the new inputs unless the old one can be reused. +/// If \p OldPreamble can be reused, it is returned unchanged. +/// If \p OldPreamble is null, always builds the preamble. +/// If \p PreambleCallback is set, it will be run on top of the AST while +/// building the preamble. Note that if the old preamble was reused, no AST is +/// built and, therefore, the callback will not be executed. +std::shared_ptr +buildPreamble(PathRef FileName, CompilerInvocation &CI, + std::shared_ptr OldPreamble, + const tooling::CompileCommand &OldCompileCommand, + const ParseInputs &Inputs, + std::shared_ptr PCHs, bool StoreInMemory, + PreambleParsedCallback PreambleCallback); + +/// Build an AST from provided user inputs. This function does not check if +/// preamble can be reused, as this function expects that \p Preamble is the +/// result of calling buildPreamble. +llvm::Optional +buildAST(PathRef FileName, std::unique_ptr Invocation, + const ParseInputs &Inputs, + std::shared_ptr Preamble, + std::shared_ptr PCHs); /// Get the beginning SourceLocation at a specified \p Pos. /// May be invalid if Pos is, or if there's no identifier. Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -175,8 +175,12 @@ ASTDiags.EndSourceFile(); std::vector ParsedDecls = Action->takeTopLevelDecls(); + std::vector Diags = ASTDiags.take(); + // Add diagnostics from the preamble, if any. + if (Preamble) + Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end()); return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action), - std::move(ParsedDecls), ASTDiags.take(), + std::move(ParsedDecls), std::move(Diags), std::move(Inclusions)); } @@ -243,120 +247,57 @@ assert(this->Action); } -CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory, - std::shared_ptr PCHs, - PreambleParsedCallback PreambleCallback) - : FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory), - PCHs(std::move(PCHs)), PreambleCallback(std::move(PreambleCallback)) { - log("Created CppFile for " + FileName); -} - -llvm::Optional> CppFile::rebuild(ParseInputs &&Inputs) { - log("Rebuilding file " + FileName + " with command [" + - Inputs.CompileCommand.Directory + "] " + - llvm::join(Inputs.CompileCommand.CommandLine, " ")); - +std::unique_ptr +clangd::buildCompilerInvocation(const ParseInputs &Inputs) { std::vector ArgStrs; for (const auto &S : Inputs.CompileCommand.CommandLine) ArgStrs.push_back(S.c_str()); if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) { - log("Couldn't set working directory"); - // We run parsing anyway, our lit-tests rely on results for non-existing - // working dirs. - } - - // Prepare CompilerInvocation. - std::unique_ptr CI; - { - // FIXME(ibiryukov): store diagnostics from CommandLine when we start - // reporting them. - IgnoreDiagnostics IgnoreDiagnostics; - IntrusiveRefCntPtr CommandLineDiagsEngine = - CompilerInstance::createDiagnostics(new DiagnosticOptions, - &IgnoreDiagnostics, false); - CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine, - Inputs.FS); - if (!CI) { - log("Could not build CompilerInvocation for file " + FileName); - AST = llvm::None; - Preamble = nullptr; - return llvm::None; - } - // createInvocationFromCommandLine sets DisableFree. - CI->getFrontendOpts().DisableFree = false; - CI->getLangOpts()->CommentOpts.ParseAllComments = true; - } - - std::unique_ptr ContentsBuffer = - llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, FileName); - - // Compute updated Preamble. - std::shared_ptr NewPreamble = - rebuildPreamble(*CI, Inputs.CompileCommand, Inputs.FS, *ContentsBuffer); - - // Remove current AST to avoid wasting memory. - AST = llvm::None; - // Compute updated AST. - llvm::Optional NewAST; - { - trace::Span Tracer("Build"); - SPAN_ATTACH(Tracer, "File", FileName); - NewAST = ParsedAST::Build(std::move(CI), NewPreamble, - std::move(ContentsBuffer), PCHs, Inputs.FS); - } - - std::vector Diagnostics; - if (NewAST) { - // Collect diagnostics from both the preamble and the AST. - if (NewPreamble) - Diagnostics = NewPreamble->Diags; - Diagnostics.insert(Diagnostics.end(), NewAST->getDiagnostics().begin(), - NewAST->getDiagnostics().end()); - } - - // Write the results of rebuild into class fields. - Command = std::move(Inputs.CompileCommand); - Preamble = std::move(NewPreamble); - AST = std::move(NewAST); - return Diagnostics; -} - -const std::shared_ptr &CppFile::getPreamble() const { - return Preamble; -} - -ParsedAST *CppFile::getAST() const { - // We could add mutable to AST instead of const_cast here, but that would also - // allow writing to AST from const methods. - return AST ? const_cast(AST.getPointer()) : nullptr; -} - -std::size_t CppFile::getUsedBytes() const { - std::size_t Total = 0; - if (AST) - Total += AST->getUsedBytes(); - if (StorePreamblesInMemory && Preamble) - Total += Preamble->Preamble.getSize(); - return Total; -} - -std::shared_ptr -CppFile::rebuildPreamble(CompilerInvocation &CI, - const tooling::CompileCommand &Command, - IntrusiveRefCntPtr FS, - llvm::MemoryBuffer &ContentsBuffer) const { - const auto &OldPreamble = this->Preamble; - auto Bounds = ComputePreambleBounds(*CI.getLangOpts(), &ContentsBuffer, 0); - if (OldPreamble && compileCommandsAreEqual(this->Command, Command) && - OldPreamble->Preamble.CanReuse(CI, &ContentsBuffer, Bounds, FS.get())) { + log("Couldn't set working directory when creating compiler invocation."); + // We proceed anyway, our lit-tests rely on results for non-existing working + // dirs. + } + + // FIXME(ibiryukov): store diagnostics from CommandLine when we start + // reporting them. + IgnoreDiagnostics IgnoreDiagnostics; + IntrusiveRefCntPtr CommandLineDiagsEngine = + CompilerInstance::createDiagnostics(new DiagnosticOptions, + &IgnoreDiagnostics, false); + std::unique_ptr CI = createInvocationFromCommandLine( + ArgStrs, CommandLineDiagsEngine, Inputs.FS); + if (!CI) + return nullptr; + // createInvocationFromCommandLine sets DisableFree. + CI->getFrontendOpts().DisableFree = false; + CI->getLangOpts()->CommentOpts.ParseAllComments = true; + return CI; +} + +std::shared_ptr clangd::buildPreamble( + PathRef FileName, CompilerInvocation &CI, + std::shared_ptr OldPreamble, + const tooling::CompileCommand &OldCompileCommand, const ParseInputs &Inputs, + std::shared_ptr PCHs, bool StoreInMemory, + PreambleParsedCallback PreambleCallback) { + // Note that we don't need to copy the input contents, preamble can live + // without those. + auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Inputs.Contents); + auto Bounds = + ComputePreambleBounds(*CI.getLangOpts(), ContentsBuffer.get(), 0); + + if (OldPreamble && + compileCommandsAreEqual(Inputs.CompileCommand, OldCompileCommand) && + OldPreamble->Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds, + Inputs.FS.get())) { log("Reusing preamble for file " + Twine(FileName)); return OldPreamble; } log("Preamble for file " + Twine(FileName) + " cannot be reused. Attempting to rebuild it."); - trace::Span Tracer("Preamble"); + trace::Span Tracer("BuildPreamble"); SPAN_ATTACH(Tracer, "File", FileName); StoreDiags PreambleDiagnostics; IntrusiveRefCntPtr PreambleDiagsEngine = @@ -369,9 +310,14 @@ CI.getFrontendOpts().SkipFunctionBodies = true; 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 BuiltPreamble = PrecompiledPreamble::Build( - CI, &ContentsBuffer, Bounds, *PreambleDiagsEngine, FS, PCHs, - /*StoreInMemory=*/StorePreamblesInMemory, SerializedDeclsCollector); + CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Inputs.FS, PCHs, + StoreInMemory, SerializedDeclsCollector); // When building the AST for the main file, we do want the function // bodies. @@ -380,7 +326,6 @@ if (BuiltPreamble) { log("Built preamble of size " + Twine(BuiltPreamble->getSize()) + " for file " + Twine(FileName)); - return std::make_shared( std::move(*BuiltPreamble), PreambleDiagnostics.take(), SerializedDeclsCollector.takeInclusions()); @@ -390,6 +335,24 @@ } } +llvm::Optional clangd::buildAST( + PathRef FileName, std::unique_ptr Invocation, + const ParseInputs &Inputs, std::shared_ptr Preamble, + std::shared_ptr PCHs) { + trace::Span Tracer("BuildAST"); + SPAN_ATTACH(Tracer, "File", FileName); + + 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. + } + + return ParsedAST::Build( + llvm::make_unique(*Invocation), Preamble, + llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), PCHs, Inputs.FS); +} + SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos, const FileID FID) { Index: clang-tools-extra/trunk/clangd/TUScheduler.h =================================================================== --- clang-tools-extra/trunk/clangd/TUScheduler.h +++ clang-tools-extra/trunk/clangd/TUScheduler.h @@ -42,6 +42,15 @@ /// within a bounded amount of time. }; +/// Configuration of the AST retention policy. This only covers retention of +/// *idle* ASTs. If queue has operations requiring the AST, they might be +/// kept in memory. +struct ASTRetentionPolicy { + /// Maximum number of ASTs to be retained in memory when there are no pending + /// requests for them. + unsigned MaxRetainedASTs = 3; +}; + /// Handles running tasks for ClangdServer and managing the resources (e.g., /// preambles and ASTs) for opened files. /// TUScheduler is not thread-safe, only one thread should be providing updates @@ -53,13 +62,19 @@ public: TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, PreambleParsedCallback PreambleCallback, - std::chrono::steady_clock::duration UpdateDebounce); + std::chrono::steady_clock::duration UpdateDebounce, + ASTRetentionPolicy RetentionPolicy); ~TUScheduler(); /// Returns estimated memory usage for each of the currently open files. /// The order of results is unspecified. std::vector> getUsedBytesPerFile() const; + /// Returns a list of files with ASTs currently stored in memory. This method + /// is not very reliable and is only used for test. E.g., the results will not + /// contain files that currently run something over their AST. + std::vector getFilesWithCachedAST() const; + /// Schedule an update for \p File. Adds \p File to a list of tracked files if /// \p File was not part of it before. /// FIXME(ibiryukov): remove the callback from this function. @@ -99,11 +114,18 @@ /// This class stores per-file data in the Files map. struct FileData; +public: + /// Responsible for retaining and rebuilding idle ASTs. An implementation is + /// an LRU cache. + class ASTCache; + +private: const bool StorePreamblesInMemory; const std::shared_ptr PCHOps; const PreambleParsedCallback PreambleCallback; Semaphore Barrier; llvm::StringMap> Files; + std::unique_ptr IdleASTs; // None when running tasks synchronously and non-None when running tasks // asynchronously. llvm::Optional PreambleTasks; Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp =================================================================== --- clang-tools-extra/trunk/clangd/TUScheduler.cpp +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp @@ -45,9 +45,12 @@ #include "TUScheduler.h" #include "Logger.h" #include "Trace.h" +#include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PCHContainerOperations.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Support/Errc.h" #include "llvm/Support/Path.h" +#include #include #include #include @@ -55,6 +58,76 @@ namespace clang { namespace clangd { using std::chrono::steady_clock; + +namespace { +class ASTWorker; +} + +/// An LRU cache of idle ASTs. +/// Because we want to limit the overall number of these we retain, the cache +/// owns ASTs (and may evict them) while their workers are idle. +/// Workers borrow ASTs when active, and return them when done. +class TUScheduler::ASTCache { +public: + using Key = const ASTWorker *; + + ASTCache(unsigned MaxRetainedASTs) : MaxRetainedASTs(MaxRetainedASTs) {} + + /// Returns result of getUsedBytes() for the AST cached by \p K. + /// If no AST is cached, 0 is returned. + bool getUsedBytes(Key K) { + std::lock_guard Lock(Mut); + auto It = findByKey(K); + if (It == LRU.end() || !It->second) + return 0; + return It->second->getUsedBytes(); + } + + /// Store the value in the pool, possibly removing the last used AST. + /// The value should not be in the pool when this function is called. + void put(Key K, std::unique_ptr V) { + std::unique_lock Lock(Mut); + assert(findByKey(K) == LRU.end()); + + LRU.insert(LRU.begin(), {K, std::move(V)}); + if (LRU.size() <= MaxRetainedASTs) + return; + // We're past the limit, remove the last element. + std::unique_ptr ForCleanup = std::move(LRU.back().second); + LRU.pop_back(); + // Run the expensive destructor outside the lock. + Lock.unlock(); + ForCleanup.reset(); + } + + /// Returns the cached value for \p K, or llvm::None if the value is not in + /// the cache anymore. If nullptr was cached for \p K, this function will + /// return a null unique_ptr wrapped into an optional. + llvm::Optional> take(Key K) { + std::unique_lock Lock(Mut); + auto Existing = findByKey(K); + if (Existing == LRU.end()) + return llvm::None; + std::unique_ptr V = std::move(Existing->second); + LRU.erase(Existing); + return V; + } + +private: + using KVPair = std::pair>; + + std::vector::iterator findByKey(Key K) { + return std::find_if(LRU.begin(), LRU.end(), + [K](const KVPair &P) { return P.first == K; }); + } + + std::mutex Mut; + unsigned MaxRetainedASTs; + /// Items sorted in LRU order, i.e. first item is the most recently accessed + /// one. + std::vector LRU; /* GUARDED_BY(Mut) */ +}; + namespace { class ASTWorkerHandle; @@ -70,8 +143,12 @@ /// worker. class ASTWorker { friend class ASTWorkerHandle; - ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST, bool RunSync, - steady_clock::duration UpdateDebounce); + ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache, + Semaphore &Barrier, bool RunSync, + steady_clock::duration UpdateDebounce, + std::shared_ptr PCHs, + bool StorePreamblesInMemory, + PreambleParsedCallback PreambleCallback); public: /// Create a new ASTWorker and return a handle to it. @@ -79,9 +156,13 @@ /// is null, all requests will be processed on the calling thread /// synchronously instead. \p Barrier is acquired when processing each /// request, it is be used to limit the number of actively running threads. - static ASTWorkerHandle Create(llvm::StringRef File, AsyncTaskRunner *Tasks, - Semaphore &Barrier, CppFile AST, - steady_clock::duration UpdateDebounce); + static ASTWorkerHandle Create(PathRef FileName, + TUScheduler::ASTCache &IdleASTs, + AsyncTaskRunner *Tasks, Semaphore &Barrier, + steady_clock::duration UpdateDebounce, + std::shared_ptr PCHs, + bool StorePreamblesInMemory, + PreambleParsedCallback PreambleCallback); ~ASTWorker(); void update(ParseInputs Inputs, WantDiagnostics, @@ -92,6 +173,7 @@ std::shared_ptr getPossiblyStalePreamble() const; std::size_t getUsedBytes() const; + bool isASTCached() const; private: // Must be called exactly once on processing thread. Will return after @@ -119,22 +201,30 @@ llvm::Optional UpdateType; }; - const std::string File; + /// Handles retention of ASTs. + TUScheduler::ASTCache &IdleASTs; const bool RunSync; - // Time to wait after an update to see whether another update obsoletes it. + /// Time to wait after an update to see whether another update obsoletes it. const steady_clock::duration UpdateDebounce; + /// File that ASTWorker is reponsible for. + const Path FileName; + /// Whether to keep the built preambles in memory or on disk. + const bool StorePreambleInMemory; + /// Callback, passed to the preamble builder. + const PreambleParsedCallback PreambleCallback; + /// Helper class required to build the ASTs. + const std::shared_ptr PCHs; Semaphore &Barrier; - // AST and FileInputs are only accessed on the processing thread from run(). - CppFile AST; - // Inputs, corresponding to the current state of AST. + /// Inputs, corresponding to the current state of AST. ParseInputs FileInputs; - // Guards members used by both TUScheduler and the worker thread. + /// CompilerInvocation used for FileInputs. + std::unique_ptr Invocation; + /// Size of the last AST + /// Guards members used by both TUScheduler and the worker thread. mutable std::mutex Mutex; std::shared_ptr LastBuiltPreamble; /* GUARDED_BY(Mutex) */ - // Result of getUsedBytes() after the last rebuild or read of AST. - std::size_t LastASTSize; /* GUARDED_BY(Mutex) */ - // Set to true to signal run() to finish processing. + /// Set to true to signal run() to finish processing. bool Done; /* GUARDED_BY(Mutex) */ std::deque Requests; /* GUARDED_BY(Mutex) */ mutable std::condition_variable RequestsCV; @@ -182,27 +272,37 @@ std::shared_ptr Worker; }; -ASTWorkerHandle ASTWorker::Create(llvm::StringRef File, AsyncTaskRunner *Tasks, - Semaphore &Barrier, CppFile AST, - steady_clock::duration UpdateDebounce) { +ASTWorkerHandle ASTWorker::Create(PathRef FileName, + TUScheduler::ASTCache &IdleASTs, + AsyncTaskRunner *Tasks, Semaphore &Barrier, + steady_clock::duration UpdateDebounce, + std::shared_ptr PCHs, + bool StorePreamblesInMemory, + PreambleParsedCallback PreambleCallback) { std::shared_ptr Worker(new ASTWorker( - File, Barrier, std::move(AST), /*RunSync=*/!Tasks, UpdateDebounce)); + FileName, IdleASTs, Barrier, /*RunSync=*/!Tasks, UpdateDebounce, + std::move(PCHs), StorePreamblesInMemory, std::move(PreambleCallback))); if (Tasks) - Tasks->runAsync("worker:" + llvm::sys::path::filename(File), + Tasks->runAsync("worker:" + llvm::sys::path::filename(FileName), [Worker]() { Worker->run(); }); return ASTWorkerHandle(std::move(Worker)); } -ASTWorker::ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST, - bool RunSync, steady_clock::duration UpdateDebounce) - : File(File), RunSync(RunSync), UpdateDebounce(UpdateDebounce), - Barrier(Barrier), AST(std::move(AST)), Done(false) { - if (RunSync) - return; -} +ASTWorker::ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache, + Semaphore &Barrier, bool RunSync, + steady_clock::duration UpdateDebounce, + std::shared_ptr PCHs, + bool StorePreamblesInMemory, + PreambleParsedCallback PreambleCallback) + : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce), + FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory), + PreambleCallback(std::move(PreambleCallback)), PCHs(std::move(PCHs)), + Barrier(Barrier), Done(false) {} ASTWorker::~ASTWorker() { + // Make sure we remove the cached AST, if any. + IdleASTs.take(this); #ifndef NDEBUG std::lock_guard Lock(Mutex); assert(Done && "handle was not destroyed"); @@ -213,20 +313,41 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags, UniqueFunction)> OnUpdated) { auto Task = [=](decltype(OnUpdated) OnUpdated) mutable { + tooling::CompileCommand OldCommand = std::move(FileInputs.CompileCommand); FileInputs = Inputs; - auto Diags = AST.rebuild(std::move(Inputs)); + // Remove the old AST if it's still in cache. + IdleASTs.take(this); + + log("Updating file " + FileName + " with command [" + + Inputs.CompileCommand.Directory + "] " + + llvm::join(Inputs.CompileCommand.CommandLine, " ")); + // Rebuild the preamble and the AST. + Invocation = buildCompilerInvocation(Inputs); + if (!Invocation) { + log("Could not build CompilerInvocation for file " + FileName); + return; + } + std::shared_ptr NewPreamble = buildPreamble( + FileName, *Invocation, getPossiblyStalePreamble(), OldCommand, Inputs, + PCHs, StorePreambleInMemory, PreambleCallback); { std::lock_guard Lock(Mutex); - if (AST.getPreamble()) - LastBuiltPreamble = AST.getPreamble(); - LastASTSize = AST.getUsedBytes(); + if (NewPreamble) + LastBuiltPreamble = NewPreamble; } + // Build the AST for diagnostics. + llvm::Optional AST = + buildAST(FileName, llvm::make_unique(*Invocation), + Inputs, NewPreamble, PCHs); // We want to report the diagnostics even if this update was cancelled. // It seems more useful than making the clients wait indefinitely if they // spam us with updates. - if (Diags && WantDiags != WantDiagnostics::No) - OnUpdated(std::move(*Diags)); + if (WantDiags != WantDiagnostics::No && AST) + OnUpdated(AST->getDiagnostics()); + // Stash the AST in the cache for further use. + IdleASTs.put(this, + AST ? llvm::make_unique(std::move(*AST)) : nullptr); }; startTask("Update", Bind(Task, std::move(OnUpdated)), WantDiags); @@ -236,20 +357,26 @@ llvm::StringRef Name, UniqueFunction)> Action) { auto Task = [=](decltype(Action) Action) { - ParsedAST *ActualAST = AST.getAST(); - if (!ActualAST) { - Action(llvm::make_error("invalid AST", - llvm::errc::invalid_argument)); - return; + llvm::Optional> AST = IdleASTs.take(this); + if (!AST) { + // Try rebuilding the AST. + llvm::Optional NewAST = + Invocation + ? buildAST(FileName, + llvm::make_unique(*Invocation), + FileInputs, getPossiblyStalePreamble(), PCHs) + : llvm::None; + AST = NewAST ? llvm::make_unique(std::move(*NewAST)) : nullptr; } - Action(InputsAndAST{FileInputs, *ActualAST}); - - // Size of the AST might have changed after reads too, e.g. if some decls - // were deserialized from preamble. - std::lock_guard Lock(Mutex); - LastASTSize = ActualAST->getUsedBytes(); + // Make sure we put the AST back into the LRU cache. + auto _ = llvm::make_scope_exit( + [&AST, this]() { IdleASTs.put(this, std::move(*AST)); }); + // Run the user-provided action. + if (!*AST) + return Action(llvm::make_error( + "invalid AST", llvm::errc::invalid_argument)); + Action(InputsAndAST{FileInputs, **AST}); }; - startTask(Name, Bind(Task, std::move(Action)), /*UpdateType=*/llvm::None); } @@ -261,10 +388,17 @@ } std::size_t ASTWorker::getUsedBytes() const { - std::lock_guard Lock(Mutex); - return LastASTSize; + // Note that we don't report the size of ASTs currently used for processing + // the in-flight requests. We used this information for debugging purposes + // only, so this should be fine. + std::size_t Result = IdleASTs.getUsedBytes(this); + if (auto Preamble = getPossiblyStalePreamble()) + Result += Preamble->Preamble.getSize(); + return Result; } +bool ASTWorker::isASTCached() const { return IdleASTs.getUsedBytes(this) != 0; } + void ASTWorker::stop() { { std::lock_guard Lock(Mutex); @@ -278,7 +412,7 @@ llvm::Optional UpdateType) { if (RunSync) { assert(!Done && "running a task after stop()"); - trace::Span Tracer(Name + ":" + llvm::sys::path::filename(File)); + trace::Span Tracer(Name + ":" + llvm::sys::path::filename(FileName)); Task(); return; } @@ -415,10 +549,12 @@ TUScheduler::TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, PreambleParsedCallback PreambleCallback, - steady_clock::duration UpdateDebounce) + std::chrono::steady_clock::duration UpdateDebounce, + ASTRetentionPolicy RetentionPolicy) : StorePreamblesInMemory(StorePreamblesInMemory), PCHOps(std::make_shared()), PreambleCallback(std::move(PreambleCallback)), Barrier(AsyncThreadsCount), + IdleASTs(llvm::make_unique(RetentionPolicy.MaxRetainedASTs)), UpdateDebounce(UpdateDebounce) { if (0 < AsyncThreadsCount) { PreambleTasks.emplace(); @@ -454,9 +590,9 @@ if (!FD) { // Create a new worker to process the AST-related tasks. ASTWorkerHandle Worker = ASTWorker::Create( - File, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier, - CppFile(File, StorePreamblesInMemory, PCHOps, PreambleCallback), - UpdateDebounce); + File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr, + Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, + PreambleCallback); FD = std::unique_ptr(new FileData{ Inputs.Contents, Inputs.CompileCommand, std::move(Worker)}); } else { @@ -538,5 +674,15 @@ return Result; } +std::vector TUScheduler::getFilesWithCachedAST() const { + std::vector Result; + for (auto &&PathAndFile : Files) { + if (!PathAndFile.second->Worker->isASTCached()) + continue; + Result.push_back(PathAndFile.first()); + } + return Result; +} + } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/test/clangd/trace.test =================================================================== --- clang-tools-extra/trunk/test/clangd/trace.test +++ clang-tools-extra/trunk/test/clangd/trace.test @@ -8,14 +8,14 @@ # CHECK: "args": { # CHECK: "File": "{{.*(/|\\)}}foo.c" # CHECK: }, -# CHECK: "name": "Preamble", +# CHECK: "name": "BuildPreamble", # CHECK: "ph": "X", # CHECK: } # CHECK: { # CHECK: "args": { # CHECK: "File": "{{.*(/|\\)}}foo.c" # CHECK: }, -# CHECK: "name": "Build", +# CHECK: "name": "BuildAST", # CHECK: "ph": "X", # CHECK: } # CHECK: }, Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp @@ -11,6 +11,7 @@ #include "TestFS.h" #include "TestTU.h" #include "index/FileIndex.h" +#include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CompilationDatabase.h" @@ -208,18 +209,6 @@ TEST(FileIndexTest, RebuildWithPreamble) { auto FooCpp = testPath("foo.cpp"); auto FooH = testPath("foo.h"); - FileIndex Index; - bool IndexUpdated = false; - CppFile File("foo.cpp", /*StorePreambleInMemory=*/true, - std::make_shared(), - [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx, - std::shared_ptr PP) { - EXPECT_FALSE(IndexUpdated) - << "Expected only a single index update"; - IndexUpdated = true; - Index.update(FilePath, &Ctx, std::move(PP)); - }); - // Preparse ParseInputs. ParseInputs PI; PI.CompileCommand.Directory = testRoot(); @@ -243,7 +232,19 @@ )cpp"; // Rebuild the file. - File.rebuild(std::move(PI)); + auto CI = buildCompilerInvocation(PI); + + FileIndex Index; + bool IndexUpdated = false; + buildPreamble( + FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI, + std::make_shared(), /*StoreInMemory=*/true, + [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx, + std::shared_ptr PP) { + EXPECT_FALSE(IndexUpdated) << "Expected only a single index update"; + IndexUpdated = true; + Index.update(FilePath, &Ctx, std::move(PP)); + }); ASSERT_TRUE(IndexUpdated); // Check the index contains symbols from the preamble, but not from the main Index: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp @@ -18,8 +18,11 @@ namespace clang { namespace clangd { +using ::testing::_; +using ::testing::AnyOf; using ::testing::Pair; using ::testing::Pointee; +using ::testing::UnorderedElementsAre; void ignoreUpdate(llvm::Optional>) {} void ignoreError(llvm::Error Err) { @@ -43,7 +46,8 @@ TUScheduler S(getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, - /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero()); + /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), + ASTRetentionPolicy()); auto Added = testPath("added.cpp"); Files[Added] = ""; @@ -99,7 +103,8 @@ getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, - /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero()); + /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), + ASTRetentionPolicy()); auto Path = testPath("foo.cpp"); S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes, [&](std::vector) { Ready.wait(); }); @@ -127,7 +132,8 @@ TUScheduler S(getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, - /*UpdateDebounce=*/std::chrono::seconds(1)); + /*UpdateDebounce=*/std::chrono::seconds(1), + ASTRetentionPolicy()); // FIXME: we could probably use timeouts lower than 1 second here. auto Path = testPath("foo.cpp"); S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto, @@ -158,7 +164,8 @@ TUScheduler S(getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, - /*UpdateDebounce=*/std::chrono::milliseconds(50)); + /*UpdateDebounce=*/std::chrono::milliseconds(50), + ASTRetentionPolicy()); std::vector Files; for (int I = 0; I < FilesCount; ++I) { @@ -219,18 +226,18 @@ { WithContextValue WithNonce(NonceKey, ++Nonce); - S.runWithPreamble( - "CheckPreamble", File, - [Inputs, Nonce, &Mut, &TotalPreambleReads]( - llvm::Expected Preamble) { - EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce)); - - ASSERT_TRUE((bool)Preamble); - EXPECT_EQ(Preamble->Contents, Inputs.Contents); - - std::lock_guard Lock(Mut); - ++TotalPreambleReads; - }); + S.runWithPreamble("CheckPreamble", File, + [Inputs, Nonce, &Mut, &TotalPreambleReads]( + llvm::Expected Preamble) { + EXPECT_THAT(Context::current().get(NonceKey), + Pointee(Nonce)); + + ASSERT_TRUE((bool)Preamble); + EXPECT_EQ(Preamble->Contents, Inputs.Contents); + + std::lock_guard Lock(Mut); + ++TotalPreambleReads; + }); } } } @@ -242,5 +249,55 @@ EXPECT_EQ(TotalPreambleReads, FilesCount * UpdatesPerFile); } +TEST_F(TUSchedulerTests, EvictedAST) { + ASTRetentionPolicy Policy; + Policy.MaxRetainedASTs = 2; + TUScheduler S( + /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true, + PreambleParsedCallback(), + /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy); + + llvm::StringLiteral SourceContents = R"cpp( + int* a; + double* b = a; + )cpp"; + + auto Foo = testPath("foo.cpp"); + auto Bar = testPath("bar.cpp"); + auto Baz = testPath("baz.cpp"); + + std::atomic BuiltASTCounter; + BuiltASTCounter = false; + // Build one file in advance. We will not access it later, so it will be the + // one that the cache will evict. + S.update(Foo, getInputs(Foo, SourceContents), WantDiagnostics::Yes, + [&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1))); + ASSERT_EQ(BuiltASTCounter.load(), 1); + + // Build two more files. Since we can retain only 2 ASTs, these should be the + // ones we see in the cache later. + S.update(Bar, getInputs(Bar, SourceContents), WantDiagnostics::Yes, + [&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; }); + S.update(Baz, getInputs(Baz, SourceContents), WantDiagnostics::Yes, + [&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1))); + ASSERT_EQ(BuiltASTCounter.load(), 3); + + // Check only the last two ASTs are retained. + ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz)); + + // Access the old file again. + S.update(Foo, getInputs(Foo, SourceContents), WantDiagnostics::Yes, + [&BuiltASTCounter](std::vector Diags) { ++BuiltASTCounter; }); + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1))); + ASSERT_EQ(BuiltASTCounter.load(), 4); + + // Check the AST for foo.cpp is retained now and one of the others got + // evicted. + EXPECT_THAT(S.getFilesWithCachedAST(), + UnorderedElementsAre(Foo, AnyOf(Bar, Baz))); +} + } // namespace clangd } // namespace clang