[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kuganv updated this revision to Diff 521936. kuganv edited the summary of this revision. kuganv added a comment. Herald added a project: clang. Re-implemented based on review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148088/new/

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kuganv added a comment. In D148088#4316352 , @kadircet wrote: > Sorry I was trying to give some brief idea about what it might look like in > `Implementation Concerns` section above, but let me give some more details; > > I think we can just change the

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kadircet added a comment. Sorry I was trying to give some brief idea about what it might look like in `Implementation Concerns` section above, but let me give some more details; I think we can just change the signature for PreambleParsedCallback

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kuganv added a comment. Thanks @kadircet , @sammccall and @ilya-biryukov for the super detailed explanation. As mentioned, we will test clangd with an ASAN build to test. I also would like to get your feedback on the implementation. After we claim the AST as per @sammccall's patch into

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kadircet added a comment. Thanks for the patch, this is a topic we've discussed a couple times in the past as well as it has far reaching implications. I believe we're actually in a much better situation nowadays. To summarise some concerns (thanks to @sammccall for useful discussion);

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kuganv marked an inline comment as done. kuganv added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:88 +if (PreambleIndexTask) + PreambleIndexTask->runAsync("task:" + Path + Version, + std::move(Task));

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

sammccall added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:88 +if (PreambleIndexTask) + PreambleIndexTask->runAsync("task:" + Path + Version, + std::move(Task)); ilya-biryukov wrote: > This

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

ivanmurashko added a comment. In D148088#4302269 , @ilya-biryukov wrote: > > Could you elaborate a bit more on what is being cached with modules and how > this patch would affect it? I hope that I can provide some info here. We are going to use

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

ilya-biryukov added a comment. In D148088#4302182 , @kuganv wrote: > 1. We see preamble indexing taking as much as 18% of the time for some files. > Moving preamble indexing out of the critical path helps there. Which operation are you measuring? 18%

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kuganv added a comment. In D148088#4302092 , @ilya-biryukov wrote: >> We would like to move the preamble index out of the critical path > > Could you provide motivation why you need to do it? What is the underlying > problem that this change is trying

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

ilya-biryukov added a comment. > We would like to move the preamble index out of the critical path Could you provide motivation why you need to do it? What is the underlying problem that this change is trying to solve? We rely on preamble being indexed at that particular time for correct

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

kuganv created this revision. Herald added subscribers: kadircet, arphaman, javed.absar. Herald added a project: All. kuganv updated this revision to Diff 517147. kuganv added a comment. kuganv edited the summary of this revision. kuganv added reviewers: kadircet, sam, ilya-biryukov, nridge.