[lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

[bulbazord]

Summary

DWARFAttribute is used in 2 classes: DWARFAbbreviationDecl and
DWARFAttributes. The former stores a std::vector of them and the latter
has a small structure called AttributeValue that contains a
DWARFAttribute. DWARFAttributes maintains a llvm::SmallVector of
AttributeValues.

My end goal is to have DWARFAttributes have a llvm::SmallVector
specialized on DWARFAttribute. In order to do that, we'll have to move
the other elements of AttributeValue into DWARFAttribute itself. But we
don't want to do this while DWARFAbbreviationDecl is using
DWARFAttribute because it will needlessly increase the size of
DWARFAbbreviationDecl. So instead I will create a small type containing
only what DWARFAbbreviationDecl needs and call it AttributeSpec. This
is the exact same thing that LLVM does today.

I've elected to swap std::vector for llvm::SmallVector here with a pre-allocated
size of 8. I've collected time and memory measurements before this change and
after it as well. Using a c++ project with 10,000 object files and no dSYM, I
place a breakpoint by file + lineno and see how long it takes to resolve.

Before this patch:

Time (mean ± σ):     13.577 s ±  0.024 s    [User: 12.418 s, System: 1.247 s]

Total number of bytes allocated: 1.38 GiB
Total number of allocations: 6.47 million allocations

After this patch:

Time (mean ± σ):     13.287 s ±  0.020 s    [User: 12.128 s, System: 1.250 s]

Total number of bytes allocated: 1.59 GiB
Total number of allocations: 4.61 million allocations

So we consume more memory than before, but we actually make less allocations on
average.

I also measured with an llvm::SmallVector with a pre-allocated size of 4 instead
of 8 to measure how well it performs:

Time (mean ± σ):     13.246 s ±  0.048 s    [User: 12.074 s, System: 1.268 s]

Total memory consumption: 1.50 GiB
Total number of allocations: 5.74 million

Of course this data may look very different depending on the actual program
being debugged, but each of the object files had 100+ AbbreviationDeclarations
each with between 0 and 10 Attributes, so I feel this was a fair example to
consider.

[bulbazord]

I previously uploaded a similar patch that I've since abandoned. It contains valuable context so I'm linking it here for other contributors to read: D149214

[JDevlieghere]

Given the numbers I'm surprised you decided to stick with 8 rather than 4? Unless I'm reading them wrong, it seems that despite the number of allocations, it's about as fast and uses (a bit) less memory. Besides that this LGTM.

lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp

58

This could be an emplace_back.

[bulbazord]

In D150418#4337243, @JDevlieghere wrote:

Given the numbers I'm surprised you decided to stick with 8 rather than 4? Unless I'm reading them wrong, it seems that despite the number of allocations, it's about as fast and uses (a bit) less memory. Besides that this LGTM.

Yeah, I just forgot to update this before I published. Will update the patch before landing. Thanks for the review!

[bulbazord]

• Use emplace_back
• llvm::SmallVector size 8 -> 4

lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h

 
 #include "DWARFAttribute.h"
 #include "DWARFDefines.h"
 #include "SymbolFileDWARF.h"
 #include "llvm/Support/Error.h"
 
 class DWARFAbbreviationDeclaration {
 public:
+  struct AttributeSpec {
+    AttributeSpec(dw_attr_t attr, dw_form_t form, int64_t value)
+        : m_attr(attr), m_form(form), m_value(value) {}
+
+    AttributeSpec(dw_attr_t attr, dw_form_t form)
+        : m_attr(attr), m_form(form), m_value(0) {}
+
+    bool IsImplicitConst() const {
+      return m_form == lldb_private::dwarf::DW_FORM_implicit_const;
+    }
+
+    int64_t GetImplicitConstValue() const { return m_value; }
+
+    dw_attr_t GetAttribute() const { return m_attr; }
+
+    dw_form_t GetForm() const { return m_form; }
+
+  private:
+    dw_attr_t m_attr;
+    dw_form_t m_form;
+    int64_t m_value;
+  };
+
   enum { InvalidCode = 0 };
   DWARFAbbreviationDeclaration();
 
   // For hand crafting an abbreviation declaration
   DWARFAbbreviationDeclaration(dw_tag_t tag, uint8_t has_children);
 
   uint32_t Code() const { return m_code; }
   void SetCode(uint32_t code) { m_code = code; }
   dw_tag_t Tag() const { return m_tag; }
   bool HasChildren() const { return m_has_children; }
   size_t NumAttributes() const { return m_attributes.size(); }
   dw_form_t GetFormByIndex(uint32_t idx) const {
-    return m_attributes.size() > idx ? m_attributes[idx].get_form()
+    return m_attributes.size() > idx ? m_attributes[idx].GetForm()
                                      : dw_form_t(0);
   }
 
   // idx is assumed to be valid when calling GetAttrAndFormByIndex()
   void GetAttrAndFormValueByIndex(uint32_t idx, dw_attr_t &attr,
                                   DWARFFormValue &form_value) const {
-    m_attributes[idx].get(attr, form_value.FormRef(), form_value.ValueRef());
+    const AttributeSpec &spec = m_attributes[idx];
+    attr = spec.GetAttribute();
+    form_value.FormRef() = spec.GetForm();
+    if (spec.IsImplicitConst())
+      form_value.SetSigned(spec.GetImplicitConstValue());
   }
   dw_form_t GetFormByIndexUnchecked(uint32_t idx) const {
-    return m_attributes[idx].get_form();
+    return m_attributes[idx].GetForm();
   }
   uint32_t FindAttributeIndex(dw_attr_t attr) const;
 
   /// Extract one abbreviation declaration and all of its associated attributes.
   /// Possible return values:
   ///   DWARFEnumState::Complete - the extraction completed successfully.  This
   ///       was the last abbrev decl in a sequence, and the user should not call
   ///       this function again.
   ///   DWARFEnumState::MoreItems - the extraction completed successfully.  The
   ///       user should call this function again to retrieve the next decl.
   ///   llvm::Error - A parsing error occurred.  The debug info is malformed.
   llvm::Expected<lldb_private::DWARFEnumState>
   extract(const lldb_private::DWARFDataExtractor &data,
           lldb::offset_t *offset_ptr);
   bool IsValid();
 
 protected:
   uint32_t m_code = InvalidCode;
   dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
   uint8_t m_has_children = 0;
-  DWARFAttribute::collection m_attributes;
+  llvm::SmallVector<AttributeSpec, 4> m_attributes;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFABBREVIATIONDECLARATION_H

lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp

while (data.ValidOffset(*offset_ptr)) {
     // they should call this function again.
     if (!attr && !form)
       return DWARFEnumState::MoreItems;
 
     if (!attr || !form)
       return llvm::make_error<llvm::object::GenericBinaryError>(
           "malformed abbreviation declaration attribute");
 
-    DWARFFormValue::ValueType val;
-
-    if (form == DW_FORM_implicit_const)
-      val.value.sval = data.GetSLEB128(offset_ptr);
+    if (form == DW_FORM_implicit_const) {
+      int64_t value = data.GetSLEB128(offset_ptr);
+      m_attributes.emplace_back(attr, form, value);
+      continue;
+    }
 
-    m_attributes.push_back(DWARFAttribute(attr, form, val));
+    m_attributes.emplace_back(attr, form);
   }
 
   return llvm::make_error<llvm::object::GenericBinaryError>(
       "abbreviation declaration attribute list not terminated with a null "
       "entry");
 }
 
 bool DWARFAbbreviationDeclaration::IsValid() {
   return m_code != 0 && m_tag != llvm::dwarf::DW_TAG_null;
 }
 
 uint32_t
 DWARFAbbreviationDeclaration::FindAttributeIndex(dw_attr_t attr) const {
-  uint32_t i;
-  const uint32_t kNumAttributes = m_attributes.size();
-  for (i = 0; i < kNumAttributes; ++i) {
-    if (m_attributes[i].get_attr() == attr)
+  for (size_t i = 0; i < m_attributes.size(); ++i) {
+    if (m_attributes[i].GetAttribute() == attr)
       return i;
   }
   return DW_INVALID_INDEX;
 }

lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

public:
   dw_form_t get_form() const { return m_form; }
   DWARFFormValue::ValueType get_value() const { return m_value; }
   void get(dw_attr_t &attr, dw_form_t &form,
            DWARFFormValue::ValueType &val) const {
     attr = m_attr;
     form = m_form;
     val = m_value;
   }
-  typedef std::vector<DWARFAttribute> collection;
-  typedef collection::iterator iterator;
-  typedef collection::const_iterator const_iterator;
-
 protected:
   dw_attr_t m_attr;
   dw_form_t m_form;
   DWARFFormValue::ValueType m_value;
 };
 
 class DWARFAttributes {
 public: