[Lldb-commits] [PATCH] D62630: Fix a regression in DWARF access speed caused by svn revision 356190

[Greg Clayton via Phabricator via lldb-commits]

clayborg closed this revision.
clayborg marked an inline comment as done.
clayborg added a comment.

r362105 | gclayton | 2019-05-30 08:32:33 -0700 (Thu, 30 May 2019) | 13 lines

Fix a regression in DWARF access speed caused by svn revision 356190

The issue was caused by the error checking code that was added. It was incorrectly adding an extra abbreviation when DWARFEnumState::Complete was received since it would push an extra abbreviation onto the list with the abbreviation code of zero. This cause m_idx_offset in each DWARFAbbreviationDeclarationSet to be set to UINT32_MAX. This valid indicates we must linearly search for attributes, not access them in O(1) time. This caused every DWARFDebugInfoEntry that would try to get its DWARFAbbreviationDeclaration from the CU's DWARFAbbreviationDeclarationSet to always linearly search the abbreviation set for a given abbreviation code. Easy to see why this would cause things to be slow.

This regression was caused by: https://reviews.llvm.org/D59370. I asked to ensure there was no regression is parsing or access speed, but that must not have been done. In my test with 40 DWARF files trying to set a breakpoint by function name and in a header file, I see a 8% speed improvement with this fix.

There was no regression in correctness, just very inefficient access.

Added full unit testing for DWARFAbbreviationDeclarationSet parsing to ensure this doesn't regress.

Differential Revision: https://reviews.llvm.org/D62630



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:32
   dw_uleb128_t prev_abbr_code = 0;
-  DWARFEnumState state = DWARFEnumState::MoreItems;
-  while (state == DWARFEnumState::MoreItems) {
+  while (true) {
     llvm::Expected<DWARFEnumState> es =
----------------
aprantl wrote:
> Not being too familiar with the API I still find the control flow in this loop to be hard to follow. Is `offset_ptr` incremented by `extract()`?
Yes. It is passed to data.GetXXX calls and serves as the thread safe offset into the data.


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

https://reviews.llvm.org/D62630