[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

Mon Mar 14 15:03:23 PDT 2022

jingham added a comment.

This sort of change needs to do a lot more work to help the user understand what they can and can't see if they accept the speed for visibility tradeoff.

They (and we) also need to know what operations might trigger a full "hydration" of a symbol table.  It isn't useful to have all this infrastructure in place if fairly common operations trigger all the work anyway.

It also seems to me that whenever the user tells about a shared library, we should "hydrate" it.  So for instance, if I do:

(lldb) break set -s someLib.dylib -f foo.c -l 12

they you should first hydrate someLib.dylib and then set the breakpoint.

In general, there should be a strategy discussion up-front before just making this change, with use cases of what won't work (not just some disabled tests) and how ordinary users might figure out something didn't work because of the on-demand loading.  There needs to also be a way for lldb to inform users what to expect or the experience will just end up being frustrating.

It also seems like there should be a way to say "These libraries I'm unlikely to care about, but these I always want to have fully expanded".

Normally, it would be fine to say "we can add those details as we go along" but you're designing a new mode of interaction with the debugger, and I think we need to sketch out what we think are acceptable tradeoffs and where we need to give people the ability to manually intervene, etc.  So it seems like we should first be having a strategy discussion, before getting down to the details of this patch.

================
Comment at: include/lldb/Symbol/SymbolFileOnDemand.h:1
+//===-- SymbolFileOnDemand.h --------------------------------------*- C++
+//-*-===//
----------------
clayborg wrote:
> Make this on the same line and avoid the wrapping
This file seems to be out of place.  I don't think it's needed.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121631/new/

https://reviews.llvm.org/D121631