[PATCH] D87935: DebugInfo: Filter DWARFv5 TUs out of the debug_info unit list when CUs requested

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D87935#2284630 <https://reviews.llvm.org/D87935#2284630>, @jhenderson wrote:

> My instinct is that I prefer a third approach - simply keep both combined and separate lists - the combined list (could be a vector or an ordered map) is the offset-ordered list of all units, whilst there are then separate lists for compile and type units, with members being pointers to the units stored in the primary list. I think this has the advantage of being very explicit about things, with the drawback being that there are multiple members to keep in sync (possibly alleviated by having a wrapper class around the containers).

My concern there is higher memory usage/processing time - something I'm currently trying to combat. (& in fact, considering how a broader refactoring of libDebugInfoDWARF might look - to avoid parsing things a given client isn't using - I guess by that token, it might be nice to be able to read just the compile units and skip over the type units entirely - though, then if someone does ask for the debug_info units you'd need to go back and parse them to build a second list). I guess if you're OK with the lists being independently lazily built (well, not entirely independent - they would share any elements that overlap... - that'd make ownershp difficult, though), maybe it'd make sense that way. I guess alternatively I could start experimenting with a fully lazy parsing that doesn't keep a list at all - just walks the list, building one at a time... though that's going to be way more involved than I can get into right now (cross-CU referencing, etc).

> If I had to pick between the two existing options, I think I have a marginal preference for this one, since it requires less code and is less surprising to someone who might not be too familiar with the code - I would expect a list of units to be in offset order naturally, and not expect to have to go through some sort of sorting procedure in the future.

Fair - appreciate the perspective. One other wrinkle here is that if we support the "type_units()" API (I don't think anyone's actually using it, so I'd probably delete it for now) it'll need to use a fancy iterator solution too - concat_iterator + filter_iterator to take the v5 type units out of the debug_info units, and concat that with the v4 type units from debug_types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87935