[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge marked 2 inline comments as done. nridge added a comment. @sammccall, how would you feel about proceeding with the patch in its current state, with the memory usage increase brought down from 8.2% to 2.5% thanks to the combination of the simple lookup optimization + RefKind filtering,

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge updated this revision to Diff 557830. nridge added a comment. Address other review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge planned changes to this revision. nridge added a comment. The updated patch additionally implements the "simple lookup optimization" discussed in the review. With this version, memory usage on the test workload is: background_index 574MB (index 387MB, slabs 187MB). This is an increase

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge updated this revision to Diff 557829. nridge added a comment. Implement the "simple lookup optimization" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge planned changes to this revision. nridge added a comment. The updated patch implements one of the optimizations discussed during review, namely filtering the Refs stored in the RevRefs data structure to just those that could be calls. To this end, the patch introduces a new `RefKind`,

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge updated this revision to Diff 557745. nridge added a comment. Implement optimization to filter the refs stored in RevRefs to calls only Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge added a comment. To be able to reason about the impact of various memory usage optimizations, I took some baseline measurements. I used a workload consisting of the LLVM codebase, with the compile_commands.json entries filtered to those containing `clang/lib` or

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge commandeered this revision. nridge added a reviewer: qchateau. nridge added a comment. In D93829#4422090 , @qchateau wrote: > So if anyone wants to take it from there, feel free to reuse my commits (or > not). I'm going to pick up this work and

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge updated this revision to Diff 557525. nridge added a comment. Rebased to apply to recent trunk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

qchateau added a comment. I'm interested but I don't have enough free time these days. So if anyone wants to take it from there, feel free to reuse my commits (or not). If I find myself with some free time and nobody else is working on this, I'll give it a shot Repository: rG LLVM Github

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge added a comment. Thank you Sam for the suggested performance improvements, and outlining a way forward. @qchateau, are you interested in updating the patch to implement some of the described optimizations / address other comments? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

sammccall added a comment. Herald added a project: All. Time to wake up an old review! Going to put high-level thoughts on https://github.com/clangd/clangd/discussions/1206, but since one of them is memory usage I wanted to have a look at the concrete data structure here. TL;DR: I think we

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

smekkley added a comment. Herald added a project: clang-tools-extra. For what it's worth, outgoing call is useful for looking at grand children and jump immediately. Also, clangd doesn't runs on embedded devices, so extra memory allocation isn't a concern at all for most users. @sammccall which

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

qchateau added a comment. You're right, I probably got carried away for the sake of completeness. Allocating extra memory for a feature nobody is going to use is definitely not worth it. Though maybe we can take advantage of what's already been done and if we remove the `RevRefs` map, and pay

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge added a comment. I think that, as with incoming calls, the incremental value is in seeing the tree at a glance. So, you might query the outgoing calls for a function, expand the tree, and look over the transitive callees to see if the function ends up performing a certain type of

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

sammccall added a comment. Thanks for looking into this feature! I do have to ask a fairly naive question though - what's it useful for, beyond protocol completeness? The obvious alternative to a user is looking at a function's implementation, which is an extra context switch but also provides

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

qchateau added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1843 +auto Kind = Callee.SymInfo.Kind; +if (Kind != SK::Function && Kind != SK::InstanceMethod && +Kind != SK::ClassMethod && Kind != SK::StaticMethod && nridge wrote:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

nridge added a comment. Thanks for working on this! I haven't looked at the whole patch in detail, but I looked at some parts (mainly the `SymbolIndex` API and what in-memory structures we store to implement it) and wrote a few thoughts. Comment at:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

qchateau added a comment. Note: Build fails due to clang-tidy warnings in tests. I chose to keep the naming consistent with what's already in place rather than fix the clang-tidy warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

qchateau updated this revision to Diff 313776. qchateau added a comment. - Smaller reversed refs memory usage - Fix Dex::estimateMemoryUsage Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files:

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

qchateau updated this revision to Diff 313767. qchateau added a comment. - Fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93829/new/ https://reviews.llvm.org/D93829 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

qchateau created this revision. qchateau added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman, mgrang. qchateau requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. The implementation is