[Lldb-commits] [PATCH] D87675: [lldb/DWARF] Refactor to prefer emplace_back() vs. push_back()
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 16 01:18:38 PDT 2020
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Ok, this is something we can talk about. dwarf parsing is a big bottle neck, so spending some time to optimize that (and even sacrifice some readability/mantainability/etc. if needed) is worthwhile.
That said, I am pretty sure that any performance gains you're seeing (you didn't explain what/how you're measuring so it's hard to say for sure), are due to the additional inlining opportunities afforded by moving this function to the header, and _not_ because of the use of emplace_back. In my comparison <https://godbolt.org/z/5YTrqs>, the two implementations ended up looking fairly different, but I don't think the differences cannot be explained by the use of emplace_back. In both cases, the compiler was able to fully optimize away all the push_backs, constructor calls, etc., and the fast path for both functions consists of a bunch of move instructions. If one of the two versions ends up being slower, then this indicates a missed optimization opportunity on the side of the compiler (which could be removed or even reversed by using a different compiler), and not a fundamental problem with using push_back with trivially copyable structs.
_That_ said, I don't think adding the constructor is necessarily bad in this case, so I think it's fine to stick with the current implementation.
CHANGES SINCE LAST ACTION
More information about the lldb-commits