This is an archive of the discontinued LLVM Phabricator instance.

[Debuginfo][NFC] findRecursively: Replace std::vector by SmallVector
ClosedPublic

Authored by avl on Apr 29 2020, 1:35 PM.

Details

Summary

Change std::vector to SmallVector to prevent re-allocations and to
have small pre-allocated storage.

Diff Detail

Event Timeline

avl created this revision.Apr 29 2020, 1:35 PM
dblaikie accepted this revision.Apr 29 2020, 2:07 PM

Seems inoffensive - but do you have any data on this? the distribution of sizes for this container over some sample of inputs/uses?

This revision is now accepted and ready to land.Apr 29 2020, 2:07 PM
avl added a comment.Apr 29 2020, 2:51 PM

Thank you!

Seems inoffensive - but do you have any data on this? the distribution of sizes for this container over some sample of inputs/uses?

I have a performance data. For D74169 for clang it saves 1,5sec. 72sec->70.5sec. For the distribution of sizes I trusted the comment "Empirically we rarely see a depth of more than 3". I could check it and post here if neccessary...

Btw, For this function, preventing of checking for both DW_AT_abstract_origin and DW_AT_specification helps also:

@@ -376,11 +376,10 @@ DWARFDie::findRecursively(ArrayRef<dwarf::Attribute> Attrs) const {
     if (auto Value = Die.find(Attrs))
       return Value;
 
-    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_abstract_origin))
+    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_abstract_origin)) {
       if (Seen.insert(D).second)
         Worklist.push_back(D);
-
-    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_specification))
+    } else if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_specification))
       if (Seen.insert(D).second)
         Worklist.push_back(D);
   }

It looks like there should not be both of them in the same DIE. What do you think about it ?

In D79123#2011261, @avl wrote:

Thank you!

Seems inoffensive - but do you have any data on this? the distribution of sizes for this container over some sample of inputs/uses?

I have a performance data. For D74169 for clang it saves 1,5sec. 72sec->70.5sec. For the distribution of sizes I trusted the comment "Empirically we rarely see a depth of more than 3". I could check it and post here if neccessary...

Nah, don't think it's necessary, but if you're interested in the performance of this it might be worth looking into.

Btw, For this function, preventing of checking for both DW_AT_abstract_origin and DW_AT_specification helps also:

@@ -376,11 +376,10 @@ DWARFDie::findRecursively(ArrayRef<dwarf::Attribute> Attrs) const {
     if (auto Value = Die.find(Attrs))
       return Value;
 
-    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_abstract_origin))
+    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_abstract_origin)) {
       if (Seen.insert(D).second)
         Worklist.push_back(D);
-
-    if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_specification))
+    } else if (auto D = Die.getAttributeValueAsReferencedDie(DW_AT_specification))
       if (Seen.insert(D).second)
         Worklist.push_back(D);
   }

It looks like there should not be both of them in the same DIE. What do you think about it ?

Sounds reasonable to me.

clayborg accepted this revision.Apr 29 2020, 5:16 PM
This revision was automatically updated to reflect the committed changes.