[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

This revision was automatically updated to reflect the committed changes. Closed by commit rGc31367e95ce1: [clangd] Build ASTs only with fresh preambles or after building a new preamble (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 255425. kadircet marked 10 inline comments as done. kadircet added a comment. - Address comments and rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76725/new/ https://reviews.llvm.org/D76725 Files: cl

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:224 + std::vector CIDiags, WantDiagnostics WantDiags) { // Make possibly expensive copy while not holding the lock. +Request Req =

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks for your patience! This is very nice, only +100 lines on TUScheduler.cpp in the end! Comment at: clang-tools-extra/clangd/TUScheduler.cpp:224 + std:

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 254520. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76725/new/ https://reviews.llvm.org/D76725 Files: clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/Pr

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet added a comment. preambleReady part is a little bit different than you've described, it looks more like: ASTWorker::preambleReady(): enqueue({ if(preamble!=lastPreamble) { lastPreamble = preamble cache.get(); // force next read to use this preamble } build as

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 254439. kadircet marked 31 inline comments as done. kadircet added a comment. - Update comments and re-order functions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76725/new/ https://reviews.llvm.org/D76725

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

sammccall added a comment. Had some discussion offline. - having ASTWorker not worry about preamble validity before dispatching to preambleworker seems like a win - for this, preambleworker needs to call preambleReady whether it's new or not, so ASTWorker can produce diagnostics - AST reuse fro

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 253556. kadircet added a comment. - Add assertion to explicitly spell out scheduling for golden ASTs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76725/new/ https://reviews.llvm.org/D76725 Files: clang-to

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 253552. kadircet added a comment. - Make use of a separate queue for golden ASTs to prevent any races that might occur due PreambleThread finishing multiple preambles before ASTWorker gets to process others. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:623 std::string TaskName = llvm::formatv("Update ({0})", Inputs.Version); auto Task = [=]() mutable { auto RunPublish = [&](llvm::function_ref Publish) { sammccall wro

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 253347. kadircet marked 26 inline comments as done. kadircet added a comment. Herald added a subscriber: jfb. - Update description and address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76725/new/

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

sammccall added inline comments. Comment at: clang-tools-extra/clangd/TUScheduler.cpp:8 //===--===// // For each file, managed by TUScheduler, we create a single ASTWorker that +// manages an AST and a preambl

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 252841. kadircet added a comment. - Invalidate cached AST in case of preamble/input changes when building golden ASTs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76725/new/ https://reviews.llvm.org/D76725

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet added a comment. In D76725#1940282 , @sammccall wrote: > So I think we need a careful description of the new model somewhere. Not so > much on specific changes/constraints parts of the code operate under, but > what it's trying to do. > > My bes

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet updated this revision to Diff 252633. kadircet added a comment. - Build golden asts in ASTWorker thread, rather than preamble thread. - Explain new model in file comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76725/new/ https://re

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

sammccall added a comment. So I think we need a careful description of the new model somewhere. Not so much on specific changes/constraints parts of the code operate under, but what it's trying to do. My best understanding is: - In general, read requests are processed in order by the ASTWorker

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, javed.absar, ilya-biryukov. Herald added a project: clang. kadircet added a parent revision: D76304: [clangd] Update TUStatus api to accommodate prea