This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Add ConstString memory usage statistics
ClosedPublic

Authored by JDevlieghere on Jan 21 2022, 11:42 AM.

Details

Summary

Add statistics about the memory usage of the string pool. I'm particularly interested in the memory used by the allocator, i.e. the number of bytes actually used by the allocator it self as well as the number of bytes allocated through the allocator.

Diff Detail

Event Timeline

JDevlieghere requested review of this revision.Jan 21 2022, 11:42 AM
JDevlieghere created this revision.
JDevlieghere added inline comments.Jan 21 2022, 11:45 AM
lldb/include/lldb/Utility/ConstString.h
409

I purposely didn't use this function for the reasons explained in the summary. It seems like a few classes implement this, but it's not actually used anywhere. If we care, I can reimplement this in terms of the new MemoryStats. If we don't I can remove it in a follow-up patch.

kastiglione accepted this revision.Jan 21 2022, 12:12 PM
kastiglione added a subscriber: kastiglione.

nice

This revision is now accepted and ready to land.Jan 21 2022, 12:12 PM
clayborg added inline comments.Jan 21 2022, 12:47 PM
lldb/include/lldb/Utility/ConstString.h
415–416
lldb/source/Target/Statistics.cpp
72

Maybe "bytesUnallocated" instead of "bytesWasted"?

228

"constStrings" maybe? or "stringPool"? "strings" makes it seem like we are tracking all strings in LLDB

Do we want another top level key here for "memory" that we can slowly add things to? It would be nice to know how much memory symbols tables consume, DWARF data structures like the DIE vectors, and many others things. So maybe adding a "memory" key/value pair at the top here might be a good idea?

"memory": {"strings": ..., "symtab": ..., "dwarf": .... }
JDevlieghere added inline comments.Jan 21 2022, 12:59 PM
lldb/source/Target/Statistics.cpp
228

I like that

JDevlieghere marked 3 inline comments as done.

Put string statistics under memory:

"memory": {
  "strings": {
    "bytesAllocated": 2056916,
    "bytesTotal": 3145728,
    "bytesUnallocated": 1088812
  }
},
kastiglione added a comment.EditedJan 21 2022, 1:35 PM

I find "Unallocated" ambiguous. To the Allocator, maybe it's not, but to VM it is. What about Total/Used/Unused? Note that the docs for BytesAllocated say it can be used to calculated "wasted" space.

/// How many bytes we've allocated.
///
/// Used so that we can compute how much space was wasted.
size_t BytesAllocated = 0;

I find "Unallocated" ambiguous. To the Allocator, maybe it's not, but to VM it is. What about Total/Used/Unused? Note that the docs for BytesAllocated say it can be used to calculated "wasted" space.

/// How many bytes we've allocated.
///
/// Used so that we can compute how much space was wasted.
size_t BytesAllocated = 0;

I got the original terminology from the dumpStats function on the BumpPtrAllocator. I like Total/Used/Unused because there's less ambiguity.

"memory": {
  "strings": {
    "bytesTotal": 3145728,
    "bytesUnused": 1088812,
    "bytesUsed": 2056916
  }
},

@clayborg Does the latest scheme look good to you?

clayborg accepted this revision.Jan 24 2022, 2:43 PM

Yep! Looks good.

As a follow up patch it would be great to ask the SymbolFile classes to break down some memory usage. I know in the SymbolFileDWARF we have each DWARFUnit that may or may not parse the unit DIE or all of the DIEs. These data structures take up memory and we lazily try to only parse all DIEs in a DWARFUnit when we need to. If we index the debug info, we will load the info and remembers which DWARFUnits had all of their DIEs parsed, and after indexing we will free the memory to keep memory usage down.

Yep! Looks good.

As a follow up patch it would be great to ask the SymbolFile classes to break down some memory usage. I know in the SymbolFileDWARF we have each DWARFUnit that may or may not parse the unit DIE or all of the DIEs. These data structures take up memory and we lazily try to only parse all DIEs in a DWARFUnit when we need to. If we index the debug info, we will load the info and remembers which DWARFUnits had all of their DIEs parsed, and after indexing we will free the memory to keep memory usage down.

Are you referring to the m_die_array in DWARFUnit and ClearDIEsRWLocked? If so that actually looks like a good fit for the BumpPtrAllocator in which case it should be fairly easy to report.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2022, 3:13 PM