bulbazord added a comment. In D152476#4408654 <https://reviews.llvm.org/D152476#4408654>, @fdeazeve wrote:
> This is awesome! I believe you said there was no measurable perf diff? Yeah, my initial experiments measured no significant perf difference no matter how you built lldb. llvm's and lldb's `DWARFAbbreviationDeclarationSet` implementations both use llvm's `DWARFAbbreviationDeclaration` and have the same semantics around their extraction. I intentionally made both implementations behave the exact same so that this change would be as simple as possible. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:62 std::set<dw_form_t> &invalid_forms) const { - for (const auto &pair : m_abbrevCollMap) - pair.second.GetUnsupportedForms(invalid_forms); + for (const auto &pair : m_abbrevCollMap) { + for (const auto &decl : pair.second) { ---------------- aprantl wrote: > nit: LLVM coding style would probably elide the {} Will update, thanks! ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h:23 typedef std::map<dw_offset_t, DWARFAbbreviationDeclarationSet> DWARFAbbreviationDeclarationCollMap; ---------------- aprantl wrote: > for a later commit: this should probably be a DenseMap for better performance? Possibly? It would be interesting to switch away from std::map to a different type (like DenseMap) to measure performance. Either way, I also want to switch us to use `llvm::DWARFDebugAbbrev`, so it might be worth doing that work first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152476/new/ https://reviews.llvm.org/D152476 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits