clayborg added a comment. In D67022#1656874 <https://reviews.llvm.org/D67022#1656874>, @labath wrote:
> Thanks for the clarification Greg. > > Functionally, this patch seems fine, but I am still wondering if we shouldn't > make this more efficient. std::set is not the most memory-efficient > structure, and so creating a std::set entry just to store one bit seems > wasteful, particularly as there is already a container which uses the context > as a key (`m_decl_ctx_to_die`). Could just shove this bit into the > `m_decl_ctx_to_die` map (probably by changing it to something functionally > equivalent to `map<DeclContext, pair<bool, vector<DWARFDie>>>`)? Given that > the sole "raison d´être" of this map is to enable the `ParseDeclsForContext` > functionality, I don't think that having that flag be stored there should be > awkward. Quite the opposite, it would remove the awkward back-and-forth > between the SymbolFileDWARF and the DWARFASTParsers (symbol file calls > `ForEachDIEInDeclContext`, passing it a callback; then ast parser calls the > callback; finally the callback immediately re-enters the parser via > `GetDeclForUIDFromDWARF`) -- instead we could just have the SymbolFileDWARF > do > `ast_parser->PleaseParseAllDiesInThisContextIfTheyHaventBeenParsedAlready(ctx)` > and have everything happen inside the parser. > > So, overall, it seems to me that this way, we could improve the code > readability while reducing the time, and avoiding a bigger memory increase. > In fact, since we now will not be using the list of dies after they have been > parsed once, we could even free up the vector<DWARFDie> after the parsing is > complete, and thereby reduce the memory footprint as well. That would be a > win-win-win scenario. :) > > WDYT? Great idea, but this idea gave me the idea to use "m_decl_ctx_to_die" as is, and just remove the entry once we have parse all decls. Then we free the memory and don't need a bit. If there is no entry in the m_decl_ctx_to_die map, then ForEachDIEInDeclContext will just not iterate at all? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67022/new/ https://reviews.llvm.org/D67022 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
