diff --git a/src/developer/debug/zxdb/symbols/dwarf_die_decoder.cc b/src/developer/debug/zxdb/symbols/dwarf_die_decoder.cc index c267f42455a9bee7b4824d38297a8b4c723c0e33..5e89a5f0855b3314a11eea8735d66670f33f4da2 100644 --- a/src/developer/debug/zxdb/symbols/dwarf_die_decoder.cc +++ b/src/developer/debug/zxdb/symbols/dwarf_die_decoder.cc @@ -200,13 +200,12 @@ bool DwarfDieDecoder::Decode(const llvm::DWARFDie& die) { } bool DwarfDieDecoder::Decode(const llvm::DWARFDebugInfoEntry& die) { - std::vector<llvm::dwarf::Attribute> seen_attrs; - return DecodeInternal(die, kMaxAbstractOriginRefsToFollow, &seen_attrs); + seen_attrs_.clear(); + return DecodeInternal(die, kMaxAbstractOriginRefsToFollow); } bool DwarfDieDecoder::DecodeInternal( - const llvm::DWARFDebugInfoEntry& die, int abstract_origin_refs_to_follow, - std::vector<llvm::dwarf::Attribute>* seen_attrs) { + const llvm::DWARFDebugInfoEntry& die, int abstract_origin_refs_to_follow) { // This indicates the abbreviation. Each DIE starts with an abbreviation // code. The is the number that the DWARFAbbreviationDeclaration was derived // from above. We have to read it again to skip the offset over the number. @@ -263,11 +262,11 @@ bool DwarfDieDecoder::DecodeInternal( // because the typical number of attributes is small enough that this // should be more efficient than a set which requires per-element heap // allocations. - if (std::find(seen_attrs->begin(), seen_attrs->end(), spec.Attr) != - seen_attrs->end()) + if (std::find(seen_attrs_.begin(), seen_attrs_.end(), spec.Attr) != + seen_attrs_.end()) needs_dispatch = false; else - seen_attrs->push_back(spec.Attr); + seen_attrs_.push_back(spec.Attr); } if (needs_dispatch) { @@ -297,7 +296,7 @@ bool DwarfDieDecoder::DecodeInternal( // DIE "underlay" any attributes present on the current one. if (abstract_origin.isValid() && abstract_origin_refs_to_follow > 0) { return DecodeInternal(*abstract_origin.getDebugInfoEntry(), - abstract_origin_refs_to_follow - 1, seen_attrs); + abstract_origin_refs_to_follow - 1); } else { // The deepest DIE in the abstract origin chain was found (which will be // the original DIE itself if there was no abstract origin). diff --git a/src/developer/debug/zxdb/symbols/dwarf_die_decoder.h b/src/developer/debug/zxdb/symbols/dwarf_die_decoder.h index 8b0ff3920c0186b9ba22ea406b2f7633a78bc77a..1fceb4dbef13639675f35f0d76a7d8ceeb67dae9 100644 --- a/src/developer/debug/zxdb/symbols/dwarf_die_decoder.h +++ b/src/developer/debug/zxdb/symbols/dwarf_die_decoder.h @@ -143,19 +143,13 @@ class DwarfDieDecoder { // Backend for Decode() above. // - // This additionally takes a list of all attributes seen which will be added - // to (in/out var). Once seen, an attribute is not considered again. This is - // used to implement DW_AT_abstract_origin where a DIE can reference another - // one for attributes not specified. - // // Following abstract origins generates a recursive call. To prevent infinite // recursion for corrupt symbols, this function takes a maximum number of // abstract origin references to follow which is decremented each time a // recursive call is made. When this gets to 0, no more abstract origin // references will be followed. bool DecodeInternal(const llvm::DWARFDebugInfoEntry& die, - int abstract_origin_refs_to_follow, - std::vector<llvm::dwarf::Attribute>* seen_attrs); + int abstract_origin_refs_to_follow); llvm::DWARFContext* context_; llvm::DWARFUnit* unit_; @@ -170,6 +164,13 @@ class DwarfDieDecoder { // desired output location for the parent of the decoded DIE. llvm::DWARFDie* abstract_parent_ = nullptr; + // Used during decode. This should be cleared each time a new DIE is decoded + // and tracks which attributes have been seen across recursive calls of + // DecodeInternal (following abstract references). This prevents us from + // decoding the same attribute more than once across abstract origins (we + // always want the first one). + std::vector<llvm::dwarf::Attribute> seen_attrs_; + FXL_DISALLOW_COPY_AND_ASSIGN(DwarfDieDecoder); }; diff --git a/src/developer/debug/zxdb/symbols/module_symbol_index.cc b/src/developer/debug/zxdb/symbols/module_symbol_index.cc index 6418fb4cd230698903e9a8317182f9ed3f4c6da9..9147de5db24e32adf5eb5d4ef9ac64fca4f97ab3 100644 --- a/src/developer/debug/zxdb/symbols/module_symbol_index.cc +++ b/src/developer/debug/zxdb/symbols/module_symbol_index.cc @@ -266,12 +266,14 @@ class SymbolStorageIndexer { root_(root), decoder_(context, unit) { decoder_.AddCString(llvm::dwarf::DW_AT_name, &name_); + + components_.reserve(8); } void AddDIE(const SymbolStorage& impl) { // Components of the name in reverse order (so "foo::Bar::Fn") would be { // "Fn", "Bar", "foo"} - std::vector<std::string> components; + components_.clear(); // Find the declaration DIE function. Perf note: getDIEForOffset() is a // binary search. @@ -280,7 +282,7 @@ class SymbolStorageIndexer { return; // Invalid if (!FillName(die)) return; - components.emplace_back(*name_); + components_.push_back(*name_); unsigned index = unit_->getDIEIndex(die); while (true) { @@ -311,13 +313,13 @@ class SymbolStorageIndexer { if (!FillName(die)) return; // Likely corrupt, these nodes should have names. - components.emplace_back(*name_); + components_.push_back(*name_); } // Add the symbol to the index. ModuleSymbolIndexNode* cur = root_; - for (int i = static_cast<int>(components.size()) - 1; i >= 0; i--) - cur = cur->AddChild(std::move(components[i])); + for (int i = static_cast<int>(components_.size()) - 1; i >= 0; i--) + cur = cur->AddChild(components_[i]); cur->AddDie( ModuleSymbolIndexNode::DieRef(impl.ref_type, impl.entry->getOffset())); } @@ -338,6 +340,10 @@ class SymbolStorageIndexer { DwarfDieDecoder decoder_; llvm::Optional<const char*> name_; // Decoder writes into this. + + // Variable used for indexing names by AddDIE. This is not needed as a class + // member but having it here prevents reallocation for every DIE indexed. + std::vector<const char*> components_; }; } // namespace diff --git a/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc b/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc index d9e66c4b7fb682c944962e1e42bfe86fb35525a5..16d736b1b2749b1a35fd20f960c19beafd806a33 100644 --- a/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc +++ b/src/developer/debug/zxdb/symbols/module_symbol_index_node.cc @@ -85,6 +85,13 @@ ModuleSymbolIndexNode* ModuleSymbolIndexNode::AddChild(std::string&& name) { .first->second; } +ModuleSymbolIndexNode* ModuleSymbolIndexNode::AddChild(const char* name) { + return &sub_.emplace(std::piecewise_construct, + std::forward_as_tuple(name), + std::forward_as_tuple()) + .first->second; +} + void ModuleSymbolIndexNode::AddChild( std::pair<std::string, ModuleSymbolIndexNode>&& child) { auto existing = sub_.find(child.first); diff --git a/src/developer/debug/zxdb/symbols/module_symbol_index_node.h b/src/developer/debug/zxdb/symbols/module_symbol_index_node.h index eb9b002f652716dcba3b192cec03b0a2c603bcc6..92344e4cd4c282a2f21bd35336681bae794174a5 100644 --- a/src/developer/debug/zxdb/symbols/module_symbol_index_node.h +++ b/src/developer/debug/zxdb/symbols/module_symbol_index_node.h @@ -131,7 +131,13 @@ class ModuleSymbolIndexNode { // Adds a child node with the given name and returns it. If one already exits // with the name, returns the existing one. + // + // Even with move semantics, having the map construct the std::string as + // needed upon insertion is slightly faster than creating a std::string and + // passing it in. Use the char* version if the source string is a + // null-terminated C-string. ModuleSymbolIndexNode* AddChild(std::string&& name); + ModuleSymbolIndexNode* AddChild(const char* name); // Adds a child to this node. If a node with this key already exists in this // node, they will be merged. @@ -161,7 +167,7 @@ class ModuleSymbolIndexNode { // Performance note: The strings are all null-terminated C strings that come // from the mapped DWARF data. We should use that in the map instead to avoid // copying all the strings again. - std::map<std::string, ModuleSymbolIndexNode> sub_; + Map sub_; // For any DIES matching this name, lists the DIEs that implement it. // If a function or static variable has the same name as a namespace, there