clayborg added a comment.
I am concerned with increasing the size of DIERef objects. If we go to 12 bytes
per DIERef, and we mostly store these in NameToDIE in
"lldb_private::UniqueCStringMap<DIERef> m_map;" maps, which contain a
std::vector of UniqueCStringMap::Entry objects which are:
struct Entry {
ConstString cstring;
T value;
};
Where T is DIERef. This will align to 24 bytes. Before we would be at 16 when
DIERef was 8 bytes. I know we already exceeded the 8 byte size of DIERef with
the added "section" field in DIERef with SVN revision 360872, so we already
have 24 byte alignment, but the memory used by LLDB for large debug sessions
will have increased significantly since a month ago.
If DIERef becomes the same size as DWARFDIE objects, we still need DIERef as
when we index the DWARF we end up with DIERef objects and then we can unload
the actual DWARF for the compile unit (so we can't switch over to using
DWARFDIE since they have pointers to the loaded DWARF DIEs, but we will need to
watch for code that might have been using a DIERef in lieu of DWARFDIE objects
due to the DIERef size being smaller, but now that they are closer or will
usually align to the same size, some clients might be able to switch over to
use DWARFDIE objects now that there is really no size win.
So to address my concern with increase in DIERef size is wether we can use the
"lldb::user_id_t" in the NameToDie map if we can always change a user_id_t into
a DIERef and get back a size win in the process? Possibly look at anyone else
storing DIERef objects and see if they can use lldb::user_id_t values too?
================
Comment at: source/Plugins/SymbolFile/DWARF/DIERef.h:59-62
+ unsigned m_dwo_num : 31;
unsigned m_section : 1;
dw_offset_t m_unit_offset;
dw_offset_t m_die_offset;
----------------
I'm a bit concerned about increasing the size by 4 bytes here. All of our
indexes are using maps of name to DIERef objects right?
================
Comment at: source/Plugins/SymbolFile/DWARF/DIERef.h:64
};
+static_assert(sizeof(DIERef) == 12, "");
----------------
Insert a message here?
================
Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:245-246
- DIERef ref(unit.GetDebugSection(), cu_offset, die.GetOffset());
+ DIERef ref(unit.GetSymbolFileDWARF().GetDwoNum(), unit.GetDebugSection(),
+ unit.GetOffset(), die.GetOffset());
switch (tag) {
----------------
Make a DWARFDIE accessor function?:
```
DIERef DWARFDIE::GetDIERef() const;
```
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1494
+ return DebugInfo()
+ ->GetUnitAtIndex(*die_ref.dwo_num())
+ ->GetDwoSymbolFile()
----------------
Can a DWO file be missing and ever return NULL here?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1495
+ ->GetUnitAtIndex(*die_ref.dwo_num())
+ ->GetDwoSymbolFile()
+ ->GetDIE(die_ref);
----------------
NULL can never be returned here?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63428/new/
https://reviews.llvm.org/D63428
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
