[PATCH] D78950: Adds LRU caching of compile units in DWARFContext.

Tue Apr 28 06:24:10 PDT 2020

labath added a comment.

In D78950#2006434 <https://reviews.llvm.org/D78950#2006434>, @aprantl wrote:

> Do you think this is always a win, i.e., also for LLDB, or should this be done in a higher layer?

Whether this is a win or not will definitely depend on the usage patterns, and it's very hard to know that for sure before trying it out on a specific use case. My gut feeling would be that this wouldn't help lldb's memory usage much because lldb already stores so much other state, but it may have a big (negative) impact on cpu usage. However, given that this functionality can be turned off (thought that could be better documented) I don't think we need to burden ourselves with that too much with what would happen with lldb in particular.

That said, I'm not really sure what to make of this patch. What was they criterion for choosing where to place the `referCompileUnit` calls ? Right, now they seem to be present on a couple of high-level APIs, but those functions are definitely not the only way to access dwarf units. Indeed, once you start thinking about the lower level APIs, things start to get a lot more fuzzier. And dangerous -- for instance, this makes it very easy to turn a perfectly valid DWARFDie object into a landmine if some operation happens to "garbage-collect" the DWARFDebugInfoEntry object that it refers to.

I think that example would definitely speak for a separate layer with a high level api, which offers only a limited (and more controlled) way of accessing the information in the dwarf file. How feasible is that in the current situation -- I don't know.

In D78950#2006767 <https://reviews.llvm.org/D78950#2006767>, @netforce wrote:

> Although I'm not very familiar with LLDB, if this hurts the performance of it, it can opt out of using the caching by giving negative LRUSize when creating DWARFContext.

I think it's important to note here that lldb does not use (most of) llvm dwarf parsing code right now -- it has it's own semi-forked versions of a lot of stuff. However, we are looking into making it reuse more (all?) of llvm dwarf parsing code, which is why it is important to ensure the interface stays flexible enough.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78950