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 https://reviews.llvm.org/D87675/new/ https://reviews.llvm.org/D87675 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
