From 522bd2337a1804c0a4a80e1d33f5982dfe512266 Mon Sep 17 00:00:00 2001 From: Leonard Grey Date: Thu, 17 Nov 2022 11:37:26 -0500 Subject: [PATCH] Speed up testing for multiple functions at an address on Posix The way this was originally written blows up on large enough targets (like...Chromium :/). This change adds a set for amortized constant time lookup of whether a FUNC already exists at a given address. Bug: google-breakpad:751 Change-Id: I10a322da70f769c106e1e5f5b2dc3dc3f79444fd Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4031580 Reviewed-by: Mark Mentovai --- src/common/module.cc | 30 +++++++++++++----------------- src/common/module.h | 2 ++ 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/common/module.cc b/src/common/module.cc index 4c427da1..442b910d 100644 --- a/src/common/module.cc +++ b/src/common/module.cc @@ -174,26 +174,22 @@ bool Module::AddFunction(Function* function) { } } #endif - if (enable_multiple_field_) { + if (enable_multiple_field_ && function_addresses_.count(function->address)) { FunctionSet::iterator existing_function = std::find_if( functions_.begin(), functions_.end(), [&](Function* other) { return other->address == function->address; }); - if (existing_function != functions_.end()) { - (*existing_function)->is_multiple = true; - // Free the duplicate that was not inserted because this Module - // now owns it. - return false; - } - std::pair ret = functions_.insert(function); - // We just checked! - assert(ret.second); - } else { - std::pair ret = functions_.insert(function); - if (!ret.second && (*ret.first != function)) { - // Free the duplicate that was not inserted because this Module - // now owns it. - return false; - } + assert(existing_function != functions_.end()); + (*existing_function)->is_multiple = true; + // Free the duplicate that was not inserted because this Module + // now owns it. + return false; + } + function_addresses_.emplace(function->address); + std::pair ret = functions_.insert(function); + if (!ret.second && (*ret.first != function)) { + // Free the duplicate that was not inserted because this Module + // now owns it. + return false; } return true; } diff --git a/src/common/module.h b/src/common/module.h index f5031ea6..a8fcc81e 100644 --- a/src/common/module.h +++ b/src/common/module.h @@ -472,6 +472,8 @@ class Module { // point to. FileByNameMap files_; // This module's source files. FunctionSet functions_; // This module's functions. + // Used to quickly look up whether a function exists at a particular address. + unordered_set
function_addresses_; // The module owns all the call frame info entries that have been // added to it.