[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)

[Michael Buch via lldb-commits]

Michael137 wrote:

> > > > Sorry, it's been a while since I last reviewed this and I must admit I forgot about this in the mean time. The only thing I'd like to check before we can land this is to hear from @Michael137 who has been actively working on FindTypes performance recently whether this aligns or clashes with what he's been working on so we don't get a mid-air collision between patches.
> > > 
> > > 
> > > Sounds good, I look forward to hearing if this fixes the issues as well. Before this fix I ran a case where I did an expression like "MyClass::iterator" and before this fix, with linux and no accelerator tables, we would convert DWARF into clang AST types for over 2 million types, and after this fix it went down to 11. This was when debugging LLDB + LLVM + clang with debug info, so there were TONS of "iterator" entries in the manual DWARF index.
> > > On Apple the type accelerator tables have the hash of the fully qualified name which can help reduce the number of types we parse, but we don't have this on linux even with DWARF5 + .debug_names
> > 
> > 
> > To add more context to @adrian-prantl's comment, we've been working on reviving https://reviews.llvm.org/D101950, which changes the way we complete record types. The main blocker was performance, because as you mention, we often end up doing these expensive namespace searches, which gets much more noticeable with D101950. The problem we saw was mostly triggered by the second lookup that `-gsimple-template-names` does in `FindTypes`.
> > I applied your patch on top of D101950 and performance seems to be much better (by 2 orders of magnitude). It did break some tests locally for me, so I'm just skimming this patch to see what changed
> 
> All tests passed on my machine from with this, so you might try those tests without D101950 and see how things go? Happy to help if you need info on how to do things with the new lookups.

All tests are passing for me now, it had to do with some badly resolved merge conflicts

> Do we need to fix this in the commit message when I squash? Or do I just update the title of this PR?

I think when you press the `Squash and merge` button it will give you the chance to correct the title of the commit

https://github.com/llvm/llvm-project/pull/74786