[Sampling profiler] Avoid native module lookups for DEX code

Adds a function on ModuleCache to look up a module without trying to
locate and create a native module if not found, and uses this
function when looking up addresses in DEX code. This avoids an
unnecessary call to dlsym in the Android native unwinder.

Bug: 1004855
Change-Id: I8c024d47f149feeeff0d17dcfdbfd2cd36c7784a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264936
Commit-Queue: Mike Wittman <wittman@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783745}

base/profiler/module_cache.cc

@@ -33,13 +33,8 @@ ModuleCache::ModuleCache() = default;
 ModuleCache::~ModuleCache() = default;
 
 const ModuleCache::Module* ModuleCache::GetModuleForAddress(uintptr_t address) {
-  const auto non_native_module_loc = non_native_modules_.find(address);
-  if (non_native_module_loc != non_native_modules_.end())
-    return non_native_module_loc->get();
-
-  const auto native_module_loc = native_modules_.find(address);
-  if (native_module_loc != native_modules_.end())
-    return native_module_loc->get();
+  if (const ModuleCache::Module* module = GetExistingModuleForAddress(address))
+    return module;
 
   std::unique_ptr<const Module> new_module = CreateModuleForAddress(address);
   if (!new_module)
@@ -97,6 +92,19 @@ void ModuleCache::AddCustomNativeModule(std::unique_ptr<const Module> module) {
   native_modules_.insert(std::move(module));
 }
 
+const ModuleCache::Module* ModuleCache::GetExistingModuleForAddress(
+    uintptr_t address) const {
+  const auto non_native_module_loc = non_native_modules_.find(address);
+  if (non_native_module_loc != non_native_modules_.end())
+    return non_native_module_loc->get();
+
+  const auto native_module_loc = native_modules_.find(address);
+  if (native_module_loc != native_modules_.end())
+    return native_module_loc->get();
+
+  return nullptr;
+}
+
 bool ModuleCache::ModuleAndAddressCompare::operator()(
     const std::unique_ptr<const Module>& m1,
     const std::unique_ptr<const Module>& m2) const {

base/profiler/module_cache.h

@@ -98,6 +98,14 @@ class BASE_EXPORT ModuleCache {
   // will be found and added automatically when invoking GetModuleForAddress().
   void AddCustomNativeModule(std::unique_ptr<const Module> module);
 
+  // Gets the module containing |address| if one already exists, or nullptr
+  // otherwise. The returned module remains owned by and has the same lifetime
+  // as the ModuleCache object.
+  // NOTE: Only users that create their own modules and need control over native
+  // module creation should use this function. Everyone else should use
+  // GetModuleForAddress().
+  const Module* GetExistingModuleForAddress(uintptr_t address) const;
+
  private:
   // Heterogenously compares modules by base address, and modules and
   // addresses. The module/address comparison considers the address equivalent

base/profiler/native_unwinder_android.cc

@@ -202,7 +202,9 @@ UnwindResult NativeUnwinderAndroid::TryUnwind(RegisterContext* thread_context,
       regs->set_dex_pc(0);
     }
 
-    // Add the frame to |stack|.
+    // Add the frame to |stack|. Must use GetModuleForAddress rather than
+    // GetExistingModuleForAddress because the unwound-to address may be in a
+    // module associated with a different unwinder.
     const ModuleCache::Module* module =
         module_cache->GetModuleForAddress(regs->pc());
     stack->emplace_back(regs->pc(), module);
@@ -246,12 +248,13 @@ void NativeUnwinderAndroid::AddInitialModulesFromMaps(
 void NativeUnwinderAndroid::EmitDexFrame(uintptr_t dex_pc,
                                          ModuleCache* module_cache,
                                          std::vector<Frame>* stack) const {
-  const ModuleCache::Module* module = module_cache->GetModuleForAddress(dex_pc);
+  const ModuleCache::Module* module =
+      module_cache->GetExistingModuleForAddress(dex_pc);
   if (!module) {
     // The region containing |dex_pc| may not be in |module_cache| since it's
     // usually not executable (.dex file). Since non-executable regions
     // are used much less commonly, it's lazily added here instead of from
-    // AddInitialModules().
+    // AddInitialModulesFromMaps().
     unwindstack::MapInfo* map_info = memory_regions_map_->Find(dex_pc);
     if (map_info) {
       auto new_module = std::make_unique<NonElfModule>(map_info);

base/profiler/native_unwinder_android_unittest.cc

@@ -473,17 +473,10 @@ TEST(NativeUnwinderAndroidTest, MAYBE_ModuleState_SystemLibrary) {
   const uintptr_t c_library_function_address =
       reinterpret_cast<uintptr_t>(&printf);
 
-  std::vector<const ModuleCache::Module*> unwinder_modules =
-      unwinder_module_cache.GetModules();
-  const auto unwinder_module_loc = std::find_if(
-      unwinder_modules.begin(), unwinder_modules.end(),
-      [c_library_function_address](const ModuleCache::Module* module) {
-        return c_library_function_address >= module->GetBaseAddress() &&
-               c_library_function_address <
-                   module->GetBaseAddress() + module->GetSize();
-      });
-  ASSERT_NE(unwinder_modules.end(), unwinder_module_loc);
-  const ModuleCache::Module* unwinder_module = *unwinder_module_loc;
+  const ModuleCache::Module* unwinder_module =
+      unwinder_module_cache.GetExistingModuleForAddress(
+          c_library_function_address);
+  ASSERT_NE(nullptr, unwinder_module);
 
   ModuleCache reference_module_cache;
   const ModuleCache::Module* reference_module =
@@ -516,17 +509,10 @@ TEST(NativeUnwinderAndroidTest, MAYBE_ModuleState_ChromeLibrary) {
   const uintptr_t chrome_function_address =
       reinterpret_cast<uintptr_t>(&CaptureScenario);
 
-  std::vector<const ModuleCache::Module*> unwinder_modules =
-      unwinder_module_cache.GetModules();
-  const auto unwinder_module_loc = std::find_if(
-      unwinder_modules.begin(), unwinder_modules.end(),
-      [chrome_function_address](const ModuleCache::Module* module) {
-        return chrome_function_address >= module->GetBaseAddress() &&
-               chrome_function_address <
-                   module->GetBaseAddress() + module->GetSize();
-      });
-  ASSERT_NE(unwinder_modules.end(), unwinder_module_loc);
-  const ModuleCache::Module* unwinder_module = *unwinder_module_loc;
+  const ModuleCache::Module* unwinder_module =
+      unwinder_module_cache.GetExistingModuleForAddress(
+          chrome_function_address);
+  ASSERT_NE(nullptr, unwinder_module);
 
   ModuleCache reference_module_cache;
   const ModuleCache::Module* reference_module =