[Lldb-commits] [PATCH] D67022: Cache expanded CompilerDeclContext's to avoid parsing them multiple times

[Guilherme Andrade via Phabricator via lldb-commits]

guiandrade added a comment.

Thank you so much for the thorough explanation and ideas, @labath and @clayborg.

I forgot to mention that the initial motivation behind this change was to reduce those find calls as I saw they add up to a lot (10^5 calls debugging a Unreal engine sample at certain breakpoints).



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1214
+      std::make_pair(decl_ctx.GetOpaqueDeclContext(), type_system);
+  uint32_t &offset = m_decl_context_to_offset_map[offset_key];
 
----------------
clayborg wrote:
> Why the reference to 'offset'? Also we shouldn't need to make a key, just use decl_ctx:
> ```
> uint32_t offset = m_decl_context_to_offset_map[decl_ctx];
> ```
> Also, m_decl_context_to_offset_map is not populated anywhere. Not sure what this will return as this was never assigned. It will probably just return a default constructed "uint32_t"??
The idea was to use that reference to populate the map at line 1222. However, I was  only using this offset map because I thought DIEs could appear in a decl context. Given that this is not the case, I agree that using a set as you suggested is much better. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67022