[Lldb-commits] [lldb] 050c09f - [lldb][NFCI] Replace use of DWARFAttribute in DWARFAbbreviationDecl

Alex Langford via lldb-commits lldb-commits at lists.llvm.org
Fri May 12 13:13:23 PDT 2023


Author: Alex Langford
Date: 2023-05-12T13:11:56-07:00
New Revision: 050c09f0bed67f3135cb2ad99d4d35e241e5d61b

URL: https://github.com/llvm/llvm-project/commit/050c09f0bed67f3135cb2ad99d4d35e241e5d61b
DIFF: https://github.com/llvm/llvm-project/commit/050c09f0bed67f3135cb2ad99d4d35e241e5d61b.diff

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

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.

Differential Revision: https://reviews.llvm.org/D150418

Added: 
    

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
    lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
index 8dd47e6da8d5..f88a604fd916 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -53,12 +53,13 @@ DWARFAbbreviationDeclaration::extract(const DWARFDataExtractor &data,
       return llvm::make_error<llvm::object::GenericBinaryError>(
           "malformed abbreviation declaration attribute");
 
-    DWARFFormValue::ValueType val;
+    if (form == DW_FORM_implicit_const) {
+      int64_t value = data.GetSLEB128(offset_ptr);
+      m_attributes.emplace_back(attr, form, value);
+      continue;
+    }
 
-    if (form == DW_FORM_implicit_const)
-      val.value.sval = data.GetSLEB128(offset_ptr);
-
-    m_attributes.push_back(DWARFAttribute(attr, form, val));
+    m_attributes.emplace_back(attr, form);
   }
 
   return llvm::make_error<llvm::object::GenericBinaryError>(
@@ -72,10 +73,8 @@ bool DWARFAbbreviationDeclaration::IsValid() {
 
 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;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
index 1c69894f82ec..f67698dd9e62 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
@@ -16,6 +16,29 @@
 
 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();
 
@@ -28,17 +51,21 @@ class DWARFAbbreviationDeclaration {
   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;
 
@@ -59,7 +86,7 @@ class DWARFAbbreviationDeclaration {
   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

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
index faba5f5e9f6e..90e12fa02493 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -31,10 +31,6 @@ class DWARFAttribute {
     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;


        


More information about the lldb-commits mailing list