[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