Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -191,6 +191,10 @@ /// FIXME: those metrics might be useful too, we should add them. std::vector> getUsedBytesPerFile() const; + /// Returns the active dynamic index if one was built. + /// This can be useful for testing, debugging, or observing memory usage. + const SymbolIndex *dynamicIndex() const; + // Blocks the main thread until the server is idle. Only for use in tests. // Returns false if the timeout expires. LLVM_NODISCARD bool @@ -224,11 +228,10 @@ // - the dynamic index owned by ClangdServer (DynamicIdx) // - the static index passed to the constructor // - a merged view of a static and dynamic index (MergedIndex) - SymbolIndex *Index; - /// If present, an up-to-date of symbols in open files. Read via Index. + const SymbolIndex *Index; + // If present, an index of symbols in open files. Read via *Index. std::unique_ptr DynamicIdx; - // If present, a merged view of DynamicIdx and an external index. Read via - // Index. + // If present, storage for the merged static/dynamic index. Read via *Index. std::unique_ptr MergedIndex; // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex) Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -71,34 +71,57 @@ }; } // namespace -/// Manages dynamic index for open files. Each file might contribute two sets -/// of symbols to the dynamic index: symbols from the preamble and symbols -/// from the file itself. Those have different lifetimes and we merge results -/// from both -class ClangdServer::DynamicIndex : public ParsingCallbacks { +/// The dynamic index tracks symbols visible in open files. +/// For boring reasons, it doesn't implement SymbolIndex directly - use index(). +class ClangdServer::DynamicIndex { public: DynamicIndex(std::vector URISchemes) : PreambleIdx(URISchemes), MainFileIdx(URISchemes), MergedIndex(mergeIndex(&MainFileIdx, &PreambleIdx)) {} - SymbolIndex &index() const { return *MergedIndex; } + const SymbolIndex &index() const { return *MergedIndex; } - void onPreambleAST(PathRef Path, ASTContext &Ctx, - std::shared_ptr PP) override { - PreambleIdx.update(Path, &Ctx, PP, /*TopLevelDecls=*/llvm::None); - } - - void onMainAST(PathRef Path, ParsedAST &AST) override { + // Returns callbacks that can be used to update the index with new ASTs. + // Index() presents a merged view of the supplied main-file and preamble ASTs. + std::unique_ptr makeUpdateCallbacks() { + struct CB : public ParsingCallbacks { + CB(ClangdServer::DynamicIndex *This) : This(This) {} + DynamicIndex *This; + + void onPreambleAST(PathRef Path, ASTContext &Ctx, + std::shared_ptr PP) override { + This->PreambleIdx.update(Path, &Ctx, std::move(PP)); + } - MainFileIdx.update(Path, &AST.getASTContext(), AST.getPreprocessorPtr(), - AST.getLocalTopLevelDecls()); - } + void onMainAST(PathRef Path, ParsedAST &AST) override { + This->MainFileIdx.update(Path, &AST.getASTContext(), + AST.getPreprocessorPtr(), + AST.getLocalTopLevelDecls()); + } + }; + return llvm::make_unique(this); + }; private: + // Contains information from each file's preamble only. + // These are large, but update fairly infrequently (preambles are stable). + // Missing information: + // - symbol occurrences (these are always "from the main file") + // - definition locations in the main file + // + // FIXME: Because the preambles for different TUs have large overlap and + // FileIndex doesn't deduplicate, this uses lots of extra RAM. + // The biggest obstacle in fixing this: the obvious approach of partitioning + // by declaring file (rather than main file) fails if headers provide + // different symbols based on preprocessor state. FileIndex PreambleIdx; + // Contains information from each file's main AST. + // These are updated frequently (on file change), but are relatively small. + // Mostly contains: + // - occurrences of symbols declared in the preamble and referenced from main + // - symbols declared both in the main file and the preamble + // (Note that symbols *only* in the main file are not indexed). FileIndex MainFileIdx; - /// Merged view into both indexes. Merges are performed in a similar manner - /// to the merges of dynamic and static index. std::unique_ptr MergedIndex; }; @@ -127,7 +150,7 @@ // FIXME(ioeric): this can be slow and we may be able to index on less // critical paths. WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory, - DynamicIdx ? *DynamicIdx : noopParsingCallbacks(), + DynamicIdx ? DynamicIdx->makeUpdateCallbacks() : nullptr, Opts.UpdateDebounce, Opts.RetentionPolicy) { if (DynamicIdx && Opts.StaticIndex) { MergedIndex = mergeIndex(&DynamicIdx->index(), Opts.StaticIndex); @@ -144,6 +167,10 @@ // ClangdServer::DynamicIndex. ClangdServer::~ClangdServer() = default; +const SymbolIndex *ClangdServer::dynamicIndex() const { + return DynamicIdx ? &DynamicIdx->index() : nullptr; +} + void ClangdServer::setRootPath(PathRef RootPath) { auto FS = FSProvider.getFileSystem(); auto Status = FS->status(RootPath); Index: clang-tools-extra/trunk/clangd/ClangdUnit.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.h +++ clang-tools-extra/trunk/clangd/ClangdUnit.h @@ -127,8 +127,8 @@ IncludeStructure Includes; }; -using PreambleParsedCallback = std::function)>; +using PreambleParsedCallback = + std::function)>; /// Builds compiler invocation that could be used to build AST or preamble. std::unique_ptr Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -95,7 +95,7 @@ if (!ParsedCallback) return; trace::Span Tracer("Running PreambleCallback"); - ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr()); + ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr()); } void BeforeExecute(CompilerInstance &CI) override { Index: clang-tools-extra/trunk/clangd/CodeComplete.h =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.h +++ clang-tools-extra/trunk/clangd/CodeComplete.h @@ -220,11 +220,13 @@ 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, IntrusiveRefCntPtr VFS, - std::shared_ptr PCHs, SymbolIndex *Index); +SignatureHelp signatureHelp(PathRef FileName, + const tooling::CompileCommand &Command, + PrecompiledPreamble const *Preamble, + StringRef Contents, Position Pos, + IntrusiveRefCntPtr VFS, + std::shared_ptr PCHs, + const SymbolIndex *Index); // For index-based completion, we only consider: // * symbols in namespaces or translation unit scopes (e.g. no class Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -798,7 +798,7 @@ class SignatureHelpCollector final : public CodeCompleteConsumer { public: SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts, - SymbolIndex *Index, SignatureHelp &SigHelp) + const SymbolIndex *Index, SignatureHelp &SigHelp) : CodeCompleteConsumer(CodeCompleteOpts, /*OutputIsBinary=*/false), SigHelp(SigHelp), @@ -1584,7 +1584,7 @@ StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, - SymbolIndex *Index) { + const SymbolIndex *Index) { SignatureHelp Result; clang::CodeCompleteOptions Options; Options.IncludeGlobals = false; Index: clang-tools-extra/trunk/clangd/TUScheduler.h =================================================================== --- clang-tools-extra/trunk/clangd/TUScheduler.h +++ clang-tools-extra/trunk/clangd/TUScheduler.h @@ -74,8 +74,6 @@ virtual void onMainAST(PathRef Path, ParsedAST &AST) {} }; -ParsingCallbacks &noopParsingCallbacks(); - /// 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 @@ -86,7 +84,7 @@ class TUScheduler { public: TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, - ParsingCallbacks &ASTCallbacks, + std::unique_ptr ASTCallbacks, std::chrono::steady_clock::duration UpdateDebounce, ASTRetentionPolicy RetentionPolicy); ~TUScheduler(); @@ -166,7 +164,7 @@ private: const bool StorePreamblesInMemory; const std::shared_ptr PCHOps; - ParsingCallbacks &Callbacks; + std::unique_ptr Callbacks; // not nullptr Semaphore Barrier; llvm::StringMap> Files; std::unique_ptr IdleASTs; Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp =================================================================== --- clang-tools-extra/trunk/clangd/TUScheduler.cpp +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp @@ -365,13 +365,12 @@ std::shared_ptr OldPreamble = getPossiblyStalePreamble(); - std::shared_ptr NewPreamble = - buildPreamble(FileName, *Invocation, OldPreamble, OldCommand, Inputs, - PCHs, StorePreambleInMemory, - [this](PathRef Path, ASTContext &Ctx, - std::shared_ptr PP) { - Callbacks.onPreambleAST(FileName, Ctx, std::move(PP)); - }); + std::shared_ptr NewPreamble = buildPreamble( + FileName, *Invocation, OldPreamble, OldCommand, Inputs, PCHs, + StorePreambleInMemory, + [this](ASTContext &Ctx, std::shared_ptr PP) { + Callbacks.onPreambleAST(FileName, Ctx, std::move(PP)); + }); bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble); { @@ -654,11 +653,6 @@ return HardwareConcurrency; } -ParsingCallbacks &noopParsingCallbacks() { - static ParsingCallbacks *Instance = new ParsingCallbacks; - return *Instance; -} - struct TUScheduler::FileData { /// Latest inputs, passed to TUScheduler::update(). std::string Contents; @@ -668,11 +662,13 @@ TUScheduler::TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, - ParsingCallbacks &Callbacks, + std::unique_ptr Callbacks, std::chrono::steady_clock::duration UpdateDebounce, ASTRetentionPolicy RetentionPolicy) : StorePreamblesInMemory(StorePreamblesInMemory), - PCHOps(std::make_shared()), Callbacks(Callbacks), + PCHOps(std::make_shared()), + Callbacks(Callbacks ? move(Callbacks) + : llvm::make_unique()), Barrier(AsyncThreadsCount), IdleASTs(llvm::make_unique(RetentionPolicy.MaxRetainedASTs)), UpdateDebounce(UpdateDebounce) { @@ -711,7 +707,7 @@ // Create a new worker to process the AST-related tasks. ASTWorkerHandle Worker = ASTWorker::create( File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr, - Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, Callbacks); + Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, *Callbacks); FD = std::unique_ptr(new FileData{ Inputs.Contents, Inputs.CompileCommand, std::move(Worker)}); } else { 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 @@ -296,11 +296,10 @@ buildPreamble( FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI, std::make_shared(), /*StoreInMemory=*/true, - [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx, - std::shared_ptr PP) { + [&](ASTContext &Ctx, std::shared_ptr PP) { EXPECT_FALSE(IndexUpdated) << "Expected only a single index update"; IndexUpdated = true; - Index.update(FilePath, &Ctx, std::move(PP)); + Index.update(FooCpp, &Ctx, std::move(PP)); }); ASSERT_TRUE(IndexUpdated); 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 @@ -46,7 +46,7 @@ TEST_F(TUSchedulerTests, MissingFiles) { TUScheduler S(getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, noopParsingCallbacks(), + /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); @@ -104,7 +104,7 @@ Notification Ready; TUScheduler S( getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, noopParsingCallbacks(), + /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); auto Path = testPath("foo.cpp"); @@ -132,7 +132,7 @@ std::atomic CallbackCount(0); { TUScheduler S(getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, noopParsingCallbacks(), + /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::seconds(1), ASTRetentionPolicy()); // FIXME: we could probably use timeouts lower than 1 second here. @@ -165,7 +165,7 @@ Notification InconsistentReadDone; // Must live longest. TUScheduler S( getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, - noopParsingCallbacks(), + /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); auto Path = testPath("foo.cpp"); @@ -221,7 +221,7 @@ // Run TUScheduler and collect some stats. { TUScheduler S(getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, noopParsingCallbacks(), + /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::milliseconds(50), ASTRetentionPolicy()); @@ -319,7 +319,7 @@ Policy.MaxRetainedASTs = 2; TUScheduler S( /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true, - noopParsingCallbacks(), + /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy); llvm::StringLiteral SourceContents = R"cpp( @@ -369,7 +369,7 @@ TEST_F(TUSchedulerTests, EmptyPreamble) { TUScheduler S( /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true, - noopParsingCallbacks(), + /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); @@ -416,7 +416,7 @@ // the same time. All reads should get the same non-null preamble. TUScheduler S( /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true, - noopParsingCallbacks(), + /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); auto Foo = testPath("foo.cpp"); @@ -449,7 +449,7 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { TUScheduler S( /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), - /*StorePreambleInMemory=*/true, noopParsingCallbacks(), + /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); @@ -502,7 +502,7 @@ TEST_F(TUSchedulerTests, NoChangeDiags) { TUScheduler S( /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), - /*StorePreambleInMemory=*/true, noopParsingCallbacks(), + /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy());