[PATCH] D79123: [Debuginfo][NFC] findRecursively: Replace std::vector by SmallVector

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 29 16:12:55 PDT 2020


dblaikie added a comment.

In D79123#2011261 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79123/new/

https://reviews.llvm.org/D79123





More information about the llvm-commits mailing list