Commit 4ad741e8 authored by Mike Wittman's avatar Mike Wittman Committed by Commit Bot

[Sampling profiler] Make ModuleCache::Module a reference type

This is a reland of 8f219b08 and
9de3faa3 merged into a single change,
with test fixes.

* Changes ModuleCache::Module from a value type to a reference type,
  with ModuleCache maintaining ownership of the Module. ModuleCache
  needs to own its Modules to properly support Windows, which reference
  counts its modules. ModuleCache is retained as a struct to minimize
  the size of this change, but will be changed to a class in a later CL.

* Moves the parallel module caching implementation for Windows into
  ModuleCache. This is required to ensure the referenced modules outlive
  the references to them.  The parallel implementation will be merged
  with the existing module caching implementation over the course of
  later refactoring CLs.

Bug: 931418
Change-Id: Ic58afbf4f7bd48a91d0191a37b623b63501c8fa7
Reviewed-on: https://chromium-review.googlesource.com/c/1483393Reviewed-by: default avataroysteine <oysteine@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Auto-Submit: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634872}
parent 31e43994
...@@ -409,11 +409,12 @@ std::vector<Frame> NativeStackSamplerMac::RecordStackFrames( ...@@ -409,11 +409,12 @@ std::vector<Frame> NativeStackSamplerMac::RecordStackFrames(
return HasValidRbp(unwind_cursor, new_stack_top); return HasValidRbp(unwind_cursor, new_stack_top);
}; };
WalkStack(thread_state, WalkStack(
[&frames](uintptr_t frame_ip, ModuleCache::Module module) { thread_state,
frames.emplace_back(frame_ip, std::move(module)); [&frames](uintptr_t frame_ip, const ModuleCache::Module* module) {
}, frames.emplace_back(frame_ip, module);
continue_predicate); },
continue_predicate);
return frames; return frames;
} }
...@@ -444,8 +445,8 @@ bool NativeStackSamplerMac::WalkStackFromContext( ...@@ -444,8 +445,8 @@ bool NativeStackSamplerMac::WalkStackFromContext(
// libunwind adds the expected stack size, it will look for the return // libunwind adds the expected stack size, it will look for the return
// address in the wrong place. This check should ensure that we bail before // address in the wrong place. This check should ensure that we bail before
// trying to deref a bad IP obtained this way in the previous frame. // trying to deref a bad IP obtained this way in the previous frame.
const ModuleCache::Module& module = module_cache_->GetModuleForAddress(rip); const ModuleCache::Module* module = module_cache_->GetModuleForAddress(rip);
if (!module.is_valid) if (!module->is_valid)
return false; return false;
callback(static_cast<uintptr_t>(rip), module); callback(static_cast<uintptr_t>(rip), module);
......
...@@ -427,24 +427,22 @@ class NativeStackSamplerWin : public NativeStackSampler { ...@@ -427,24 +427,22 @@ class NativeStackSamplerWin : public NativeStackSampler {
win::ScopedHandle thread_handle_; win::ScopedHandle thread_handle_;
ModuleCache* module_cache_;
NativeStackSamplerTestDelegate* const test_delegate_; NativeStackSamplerTestDelegate* const test_delegate_;
// The stack base address corresponding to |thread_handle_|. // The stack base address corresponding to |thread_handle_|.
const void* const thread_stack_base_address_; const void* const thread_stack_base_address_;
// The module objects, indexed by the module handle.
std::map<HMODULE, ModuleCache::Module> module_cache_;
DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerWin); DISALLOW_COPY_AND_ASSIGN(NativeStackSamplerWin);
}; };
NativeStackSamplerWin::NativeStackSamplerWin( NativeStackSamplerWin::NativeStackSamplerWin(
win::ScopedHandle thread_handle, win::ScopedHandle thread_handle,
// Ignored for now, until we can switch the internal module cache to use
// this one.
ModuleCache* module_cache, ModuleCache* module_cache,
NativeStackSamplerTestDelegate* test_delegate) NativeStackSamplerTestDelegate* test_delegate)
: thread_handle_(thread_handle.Take()), : thread_handle_(thread_handle.Take()),
module_cache_(module_cache),
test_delegate_(test_delegate), test_delegate_(test_delegate),
thread_stack_base_address_( thread_stack_base_address_(
GetThreadEnvironmentBlock(thread_handle_.Get())->Tib.StackBase) {} GetThreadEnvironmentBlock(thread_handle_.Get())->Tib.StackBase) {}
...@@ -452,7 +450,6 @@ NativeStackSamplerWin::NativeStackSamplerWin( ...@@ -452,7 +450,6 @@ NativeStackSamplerWin::NativeStackSamplerWin(
NativeStackSamplerWin::~NativeStackSamplerWin() {} NativeStackSamplerWin::~NativeStackSamplerWin() {}
void NativeStackSamplerWin::ProfileRecordingStarting() { void NativeStackSamplerWin::ProfileRecordingStarting() {
module_cache_.clear();
} }
std::vector<Frame> NativeStackSamplerWin::RecordStackFrames( std::vector<Frame> NativeStackSamplerWin::RecordStackFrames(
...@@ -489,26 +486,8 @@ std::vector<Frame> NativeStackSamplerWin::CreateFrames( ...@@ -489,26 +486,8 @@ std::vector<Frame> NativeStackSamplerWin::CreateFrames(
frames.reserve(stack.size()); frames.reserve(stack.size());
for (const auto& frame : stack) { for (const auto& frame : stack) {
auto frame_ip = reinterpret_cast<uintptr_t>(frame.instruction_pointer); frames.emplace_back(reinterpret_cast<uintptr_t>(frame.instruction_pointer),
module_cache_->GetModuleForHandle(frame.module.Get()));
HMODULE module_handle = frame.module.Get();
if (!module_handle) {
frames.emplace_back(frame_ip, ModuleCache::Module());
continue;
}
auto loc = module_cache_.find(module_handle);
if (loc != module_cache_.end()) {
frames.emplace_back(frame_ip, loc->second);
continue;
}
ModuleCache::Module module =
ModuleCache::CreateModuleForHandle(module_handle);
if (module.is_valid)
module_cache_.insert(std::make_pair(module_handle, module));
frames.emplace_back(frame_ip, std::move(module));
} }
return frames; return frames;
......
...@@ -50,8 +50,8 @@ const int kNullProfilerId = -1; ...@@ -50,8 +50,8 @@ const int kNullProfilerId = -1;
// StackSamplingProfiler::Frame ------------------------------------- // StackSamplingProfiler::Frame -------------------------------------
StackSamplingProfiler::Frame::Frame(uintptr_t instruction_pointer, StackSamplingProfiler::Frame::Frame(uintptr_t instruction_pointer,
ModuleCache::Module module) const ModuleCache::Module* module)
: instruction_pointer(instruction_pointer), module(std::move(module)) {} : instruction_pointer(instruction_pointer), module(module) {}
StackSamplingProfiler::Frame::~Frame() = default; StackSamplingProfiler::Frame::~Frame() = default;
......
...@@ -58,14 +58,14 @@ class BASE_EXPORT StackSamplingProfiler { ...@@ -58,14 +58,14 @@ class BASE_EXPORT StackSamplingProfiler {
// This struct is only used for sampling data transfer from NativeStackSampler // This struct is only used for sampling data transfer from NativeStackSampler
// to ProfileBuilder. // to ProfileBuilder.
struct BASE_EXPORT Frame { struct BASE_EXPORT Frame {
Frame(uintptr_t instruction_pointer, ModuleCache::Module module); Frame(uintptr_t instruction_pointer, const ModuleCache::Module* module);
~Frame(); ~Frame();
// The sampled instruction pointer within the function. // The sampled instruction pointer within the function.
uintptr_t instruction_pointer; uintptr_t instruction_pointer;
// The module information. // The module information.
ModuleCache::Module module; const ModuleCache::Module* module;
}; };
// Represents parameters that configure the sampling. // Represents parameters that configure the sampling.
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "base/sampling_heap_profiler/module_cache.h" #include "base/sampling_heap_profiler/module_cache.h"
#include <utility>
#include "base/no_destructor.h" #include "base/no_destructor.h"
namespace base { namespace base {
...@@ -30,29 +32,29 @@ ModuleCache::Module::~Module() = default; ...@@ -30,29 +32,29 @@ ModuleCache::Module::~Module() = default;
ModuleCache::ModuleCache() = default; ModuleCache::ModuleCache() = default;
ModuleCache::~ModuleCache() = default; ModuleCache::~ModuleCache() = default;
const ModuleCache::Module& ModuleCache::GetModuleForAddress(uintptr_t address) { const ModuleCache::Module* ModuleCache::GetModuleForAddress(uintptr_t address) {
static NoDestructor<Module> invalid_module; static NoDestructor<Module> invalid_module;
auto it = modules_cache_map_.upper_bound(address); auto it = modules_cache_map_.upper_bound(address);
if (it != modules_cache_map_.begin()) { if (it != modules_cache_map_.begin()) {
DCHECK(!modules_cache_map_.empty()); DCHECK(!modules_cache_map_.empty());
--it; --it;
Module& module = it->second; const Module* module = it->second.get();
if (address < module.base_address + module.size) if (address < module->base_address + module->size)
return module; return module;
} }
auto module = CreateModuleForAddress(address); std::unique_ptr<Module> module = CreateModuleForAddress(address);
if (!module.is_valid) if (!module->is_valid)
return *invalid_module; return invalid_module.get();
return modules_cache_map_.emplace(module.base_address, std::move(module)) return modules_cache_map_.emplace(module->base_address, std::move(module))
.first->second; .first->second.get();
} }
std::vector<const ModuleCache::Module*> ModuleCache::GetModules() const { std::vector<const ModuleCache::Module*> ModuleCache::GetModules() const {
std::vector<const Module*> result; std::vector<const Module*> result;
result.reserve(modules_cache_map_.size()); result.reserve(modules_cache_map_.size());
for (const auto& it : modules_cache_map_) for (const auto& it : modules_cache_map_)
result.push_back(&it.second); result.push_back(it.second.get());
return result; return result;
} }
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#define BASE_SAMPLING_HEAP_PROFILER_MODULE_CACHE_H_ #define BASE_SAMPLING_HEAP_PROFILER_MODULE_CACHE_H_
#include <map> #include <map>
#include <memory>
#include <string> #include <string>
#include <vector> #include <vector>
...@@ -35,6 +36,9 @@ class BASE_EXPORT ModuleCache { ...@@ -35,6 +36,9 @@ class BASE_EXPORT ModuleCache {
size_t size); size_t size);
~Module(); ~Module();
Module(const Module&) = delete;
Module& operator=(const Module&) = delete;
// Points to the base address of the module. // Points to the base address of the module.
uintptr_t base_address; uintptr_t base_address;
...@@ -60,7 +64,10 @@ class BASE_EXPORT ModuleCache { ...@@ -60,7 +64,10 @@ class BASE_EXPORT ModuleCache {
ModuleCache(); ModuleCache();
~ModuleCache(); ~ModuleCache();
const Module& GetModuleForAddress(uintptr_t address); // Gets the module containing |address| or an invalid module if |address| is
// not within a module. The returned module remains owned by and has the same
// lifetime as the ModuleCache object.
const Module* GetModuleForAddress(uintptr_t address);
std::vector<const Module*> GetModules() const; std::vector<const Module*> GetModules() const;
private: private:
...@@ -69,7 +76,7 @@ class BASE_EXPORT ModuleCache { ...@@ -69,7 +76,7 @@ class BASE_EXPORT ModuleCache {
// Creates a Module object for the specified memory address. If the address // Creates a Module object for the specified memory address. If the address
// does not belong to a module returns an invalid module. // does not belong to a module returns an invalid module.
static Module CreateModuleForAddress(uintptr_t address); static std::unique_ptr<Module> CreateModuleForAddress(uintptr_t address);
friend class NativeStackSamplerMac; friend class NativeStackSamplerMac;
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -80,11 +87,16 @@ class BASE_EXPORT ModuleCache { ...@@ -80,11 +87,16 @@ class BASE_EXPORT ModuleCache {
#endif #endif
#if defined(OS_WIN) #if defined(OS_WIN)
static Module CreateModuleForHandle(HMODULE module_handle); const Module* GetModuleForHandle(HMODULE module_handle);
static std::unique_ptr<Module> CreateModuleForHandle(HMODULE module_handle);
friend class NativeStackSamplerWin; friend class NativeStackSamplerWin;
// The module objects, indexed by the module handle.
// TODO(wittman): Merge this state into modules_cache_map_ and remove
std::map<HMODULE, std::unique_ptr<Module>> win_module_cache_;
#endif #endif
std::map<uintptr_t, Module> modules_cache_map_; std::map<uintptr_t, std::unique_ptr<Module>> modules_cache_map_;
}; };
} // namespace base } // namespace base
......
...@@ -64,13 +64,15 @@ std::string GetUniqueId(const void* module_addr) { ...@@ -64,13 +64,15 @@ std::string GetUniqueId(const void* module_addr) {
} // namespace } // namespace
// static // static
ModuleCache::Module ModuleCache::CreateModuleForAddress(uintptr_t address) { std::unique_ptr<ModuleCache::Module> ModuleCache::CreateModuleForAddress(
uintptr_t address) {
Dl_info inf; Dl_info inf;
if (!dladdr(reinterpret_cast<const void*>(address), &inf)) if (!dladdr(reinterpret_cast<const void*>(address), &inf))
return Module(); return std::make_unique<Module>();
auto base_module_address = reinterpret_cast<uintptr_t>(inf.dli_fbase); auto base_module_address = reinterpret_cast<uintptr_t>(inf.dli_fbase);
return Module(base_module_address, GetUniqueId(inf.dli_fbase), return std::make_unique<Module>(
FilePath(inf.dli_fname), GetModuleTextSize(inf.dli_fbase)); base_module_address, GetUniqueId(inf.dli_fbase), FilePath(inf.dli_fname),
GetModuleTextSize(inf.dli_fbase));
} }
size_t ModuleCache::GetModuleTextSize(const void* module_addr) { size_t ModuleCache::GetModuleTextSize(const void* module_addr) {
......
...@@ -7,9 +7,10 @@ ...@@ -7,9 +7,10 @@
namespace base { namespace base {
// static // static
ModuleCache::Module ModuleCache::CreateModuleForAddress(uintptr_t address) { std::unique_ptr<ModuleCache::Module> ModuleCache::CreateModuleForAddress(
uintptr_t address) {
// TODO(alph): Implement it. // TODO(alph): Implement it.
return Module(); return std::make_unique<Module>();
} }
} // namespace base } // namespace base
...@@ -28,28 +28,28 @@ TEST_F(ModuleCacheTest, MAYBE_ModuleCache) { ...@@ -28,28 +28,28 @@ TEST_F(ModuleCacheTest, MAYBE_ModuleCache) {
uintptr_t ptr1 = reinterpret_cast<uintptr_t>(&AFunctionForTest); uintptr_t ptr1 = reinterpret_cast<uintptr_t>(&AFunctionForTest);
uintptr_t ptr2 = ptr1 + 1; uintptr_t ptr2 = ptr1 + 1;
ModuleCache cache; ModuleCache cache;
const ModuleCache::Module& module1 = cache.GetModuleForAddress(ptr1); const ModuleCache::Module* module1 = cache.GetModuleForAddress(ptr1);
const ModuleCache::Module& module2 = cache.GetModuleForAddress(ptr2); const ModuleCache::Module* module2 = cache.GetModuleForAddress(ptr2);
EXPECT_EQ(&module1, &module2); EXPECT_EQ(module1, module2);
EXPECT_TRUE(module1.is_valid); EXPECT_TRUE(module1->is_valid);
EXPECT_GT(module1.size, 0u); EXPECT_GT(module1->size, 0u);
EXPECT_LE(module1.base_address, ptr1); EXPECT_LE(module1->base_address, ptr1);
EXPECT_GT(module1.base_address + module1.size, ptr2); EXPECT_GT(module1->base_address + module1->size, ptr2);
} }
TEST_F(ModuleCacheTest, MAYBE_ModulesList) { TEST_F(ModuleCacheTest, MAYBE_ModulesList) {
ModuleCache cache; ModuleCache cache;
uintptr_t ptr = reinterpret_cast<uintptr_t>(&AFunctionForTest); uintptr_t ptr = reinterpret_cast<uintptr_t>(&AFunctionForTest);
const ModuleCache::Module& module = cache.GetModuleForAddress(ptr); const ModuleCache::Module* module = cache.GetModuleForAddress(ptr);
EXPECT_TRUE(module.is_valid); EXPECT_TRUE(module->is_valid);
EXPECT_EQ(1u, cache.GetModules().size()); EXPECT_EQ(1u, cache.GetModules().size());
EXPECT_EQ(&module, cache.GetModules().front()); EXPECT_EQ(module, cache.GetModules().front());
} }
TEST_F(ModuleCacheTest, InvalidModule) { TEST_F(ModuleCacheTest, InvalidModule) {
ModuleCache cache; ModuleCache cache;
const ModuleCache::Module& invalid_module = cache.GetModuleForAddress(1); const ModuleCache::Module* invalid_module = cache.GetModuleForAddress(1);
EXPECT_FALSE(invalid_module.is_valid); EXPECT_FALSE(invalid_module->is_valid);
} }
} // namespace } // namespace
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <objbase.h> #include <objbase.h>
#include <psapi.h> #include <psapi.h>
#include "base/no_destructor.h"
#include "base/process/process_handle.h" #include "base/process/process_handle.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
...@@ -66,35 +67,59 @@ void GetDebugInfoForModule(HMODULE module_handle, ...@@ -66,35 +67,59 @@ void GetDebugInfoForModule(HMODULE module_handle,
} // namespace } // namespace
// static // static
ModuleCache::Module ModuleCache::CreateModuleForAddress(uintptr_t address) { std::unique_ptr<ModuleCache::Module> ModuleCache::CreateModuleForAddress(
uintptr_t address) {
HMODULE module_handle = nullptr; HMODULE module_handle = nullptr;
if (!::GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, if (!::GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS,
reinterpret_cast<LPCTSTR>(address), reinterpret_cast<LPCTSTR>(address),
&module_handle)) { &module_handle)) {
DCHECK_EQ(ERROR_MOD_NOT_FOUND, static_cast<int>(::GetLastError())); DCHECK_EQ(ERROR_MOD_NOT_FOUND, static_cast<int>(::GetLastError()));
return Module(); return std::make_unique<Module>();
} }
Module module = CreateModuleForHandle(module_handle); std::unique_ptr<Module> module = CreateModuleForHandle(module_handle);
::CloseHandle(module_handle); ::CloseHandle(module_handle);
return module; return module;
} }
const ModuleCache::Module* ModuleCache::GetModuleForHandle(
HMODULE module_handle) {
static NoDestructor<ModuleCache::Module> invalid_module;
if (!module_handle)
return invalid_module.get();
auto loc = win_module_cache_.find(module_handle);
if (loc != win_module_cache_.end())
return loc->second.get();
std::unique_ptr<ModuleCache::Module> module =
ModuleCache::CreateModuleForHandle(module_handle);
if (module->is_valid) {
const auto result = win_module_cache_.insert(
std::make_pair(module_handle, std::move(module)));
return result.first->second.get();
}
return invalid_module.get();
}
// static // static
ModuleCache::Module ModuleCache::CreateModuleForHandle(HMODULE module_handle) { std::unique_ptr<ModuleCache::Module> ModuleCache::CreateModuleForHandle(
HMODULE module_handle) {
FilePath pdb_name; FilePath pdb_name;
std::string build_id; std::string build_id;
GetDebugInfoForModule(module_handle, &build_id, &pdb_name); GetDebugInfoForModule(module_handle, &build_id, &pdb_name);
if (build_id.empty()) if (build_id.empty())
return Module(); return std::make_unique<Module>();
MODULEINFO module_info; MODULEINFO module_info;
if (!::GetModuleInformation(GetCurrentProcessHandle(), module_handle, if (!::GetModuleInformation(GetCurrentProcessHandle(), module_handle,
&module_info, sizeof(module_info))) { &module_info, sizeof(module_info))) {
return Module(); return std::make_unique<Module>();
} }
return Module(reinterpret_cast<uintptr_t>(module_info.lpBaseOfDll), build_id, return std::make_unique<Module>(
pdb_name, module_info.SizeOfImage); reinterpret_cast<uintptr_t>(module_info.lpBaseOfDll), build_id, pdb_name,
module_info.SizeOfImage);
} }
} // namespace base } // namespace base
...@@ -130,7 +130,7 @@ void HeapProfilerController::RetrieveAndSendSnapshot() { ...@@ -130,7 +130,7 @@ void HeapProfilerController::RetrieveAndSendSnapshot() {
frames.reserve(sample.stack.size()); frames.reserve(sample.stack.size());
for (const void* frame : sample.stack) { for (const void* frame : sample.stack) {
uintptr_t address = reinterpret_cast<uintptr_t>(frame); uintptr_t address = reinterpret_cast<uintptr_t>(frame);
const base::ModuleCache::Module& module = const base::ModuleCache::Module* module =
module_cache.GetModuleForAddress(address); module_cache.GetModuleForAddress(address);
frames.emplace_back(address, module); frames.emplace_back(address, module);
} }
......
...@@ -91,22 +91,21 @@ void CallStackProfileBuilder::OnSampleCompleted( ...@@ -91,22 +91,21 @@ void CallStackProfileBuilder::OnSampleCompleted(
// keep the frame information even if its module is invalid so we have // keep the frame information even if its module is invalid so we have
// visibility into how often this issue is happening on the server. // visibility into how often this issue is happening on the server.
CallStackProfile::Location* location = stack.add_frame(); CallStackProfile::Location* location = stack.add_frame();
if (!frame.module.is_valid) if (!frame.module->is_valid)
continue; continue;
// Dedup modules. // Dedup modules.
const base::ModuleCache::Module& module = frame.module; auto module_loc = module_index_.find(frame.module);
auto module_loc = module_index_.find(module.base_address);
if (module_loc == module_index_.end()) { if (module_loc == module_index_.end()) {
modules_.push_back(module); modules_.push_back(frame.module);
size_t index = modules_.size() - 1; size_t index = modules_.size() - 1;
module_loc = module_index_.emplace(module.base_address, index).first; module_loc = module_index_.emplace(frame.module, index).first;
} }
// Write CallStackProfile::Location protobuf message. // Write CallStackProfile::Location protobuf message.
ptrdiff_t module_offset = ptrdiff_t module_offset =
reinterpret_cast<const char*>(frame.instruction_pointer) - reinterpret_cast<const char*>(frame.instruction_pointer) -
reinterpret_cast<const char*>(module.base_address); reinterpret_cast<const char*>(frame.module->base_address);
DCHECK_GE(module_offset, 0); DCHECK_GE(module_offset, 0);
location->set_address(static_cast<uint64_t>(module_offset)); location->set_address(static_cast<uint64_t>(module_offset));
location->set_module_id_index(module_loc->second); location->set_module_id_index(module_loc->second);
...@@ -165,11 +164,11 @@ void CallStackProfileBuilder::OnProfileCompleted( ...@@ -165,11 +164,11 @@ void CallStackProfileBuilder::OnProfileCompleted(
call_stack_profile->set_sampling_period_ms(sampling_period.InMilliseconds()); call_stack_profile->set_sampling_period_ms(sampling_period.InMilliseconds());
// Write CallStackProfile::ModuleIdentifier protobuf message. // Write CallStackProfile::ModuleIdentifier protobuf message.
for (const auto& module : modules_) { for (const auto* module : modules_) {
CallStackProfile::ModuleIdentifier* module_id = CallStackProfile::ModuleIdentifier* module_id =
call_stack_profile->add_module_id(); call_stack_profile->add_module_id();
module_id->set_build_id(module.id); module_id->set_build_id(module->id);
module_id->set_name_md5_prefix(HashModuleFilename(module.filename)); module_id->set_name_md5_prefix(HashModuleFilename(module->filename));
} }
PassProfilesToMetricsProvider(std::move(sampled_profile_)); PassProfilesToMetricsProvider(std::move(sampled_profile_));
......
...@@ -120,11 +120,11 @@ class CallStackProfileBuilder ...@@ -120,11 +120,11 @@ class CallStackProfileBuilder
// The indexes of stacks, indexed by stack's address. // The indexes of stacks, indexed by stack's address.
std::map<const CallStackProfile::Stack*, int, StackComparer> stack_index_; std::map<const CallStackProfile::Stack*, int, StackComparer> stack_index_;
// The indexes of modules, indexed by module's base_address. // The indexes of modules in the modules_ vector below..
std::unordered_map<uintptr_t, size_t> module_index_; std::unordered_map<const base::ModuleCache::Module*, size_t> module_index_;
// The distinct modules in the current profile. // The distinct modules in the current profile.
std::vector<base::ModuleCache::Module> modules_; std::vector<const base::ModuleCache::Module*> modules_;
// Callback made when sampling a profile completes. // Callback made when sampling a profile completes.
base::OnceClosure completed_callback_; base::OnceClosure completed_callback_;
......
...@@ -86,15 +86,15 @@ TEST(CallStackProfileBuilderTest, ProfilingCompleted) { ...@@ -86,15 +86,15 @@ TEST(CallStackProfileBuilderTest, ProfilingCompleted) {
const uintptr_t module_base_address1 = 0x1000; const uintptr_t module_base_address1 = 0x1000;
Module module1 = {module_base_address1, "1", module_path}; Module module1 = {module_base_address1, "1", module_path};
Frame frame1 = {module_base_address1 + 0x10, module1}; Frame frame1 = {module_base_address1 + 0x10, &module1};
const uintptr_t module_base_address2 = 0x1100; const uintptr_t module_base_address2 = 0x1100;
Module module2 = {module_base_address2, "2", module_path}; Module module2 = {module_base_address2, "2", module_path};
Frame frame2 = {module_base_address2 + 0x10, module2}; Frame frame2 = {module_base_address2 + 0x10, &module2};
const uintptr_t module_base_address3 = 0x1010; const uintptr_t module_base_address3 = 0x1010;
Module module3 = {module_base_address3, "3", module_path}; Module module3 = {module_base_address3, "3", module_path};
Frame frame3 = {module_base_address3 + 0x10, module3}; Frame frame3 = {module_base_address3 + 0x10, &module3};
std::vector<Frame> frames1 = {frame1, frame2}; std::vector<Frame> frames1 = {frame1, frame2};
std::vector<Frame> frames2 = {frame3}; std::vector<Frame> frames2 = {frame3};
...@@ -166,11 +166,11 @@ TEST(CallStackProfileBuilderTest, StacksDeduped) { ...@@ -166,11 +166,11 @@ TEST(CallStackProfileBuilderTest, StacksDeduped) {
const uintptr_t module_base_address1 = 0x1000; const uintptr_t module_base_address1 = 0x1000;
Module module1 = {module_base_address1, "1", module_path}; Module module1 = {module_base_address1, "1", module_path};
Frame frame1 = {module_base_address1 + 0x10, module1}; Frame frame1 = {module_base_address1 + 0x10, &module1};
const uintptr_t module_base_address2 = 0x1100; const uintptr_t module_base_address2 = 0x1100;
Module module2 = {module_base_address2, "2", module_path}; Module module2 = {module_base_address2, "2", module_path};
Frame frame2 = {module_base_address2 + 0x10, module2}; Frame frame2 = {module_base_address2 + 0x10, &module2};
std::vector<Frame> frames = {frame1, frame2}; std::vector<Frame> frames = {frame1, frame2};
...@@ -212,11 +212,11 @@ TEST(CallStackProfileBuilderTest, StacksNotDeduped) { ...@@ -212,11 +212,11 @@ TEST(CallStackProfileBuilderTest, StacksNotDeduped) {
const uintptr_t module_base_address1 = 0x1000; const uintptr_t module_base_address1 = 0x1000;
Module module1 = {module_base_address1, "1", module_path}; Module module1 = {module_base_address1, "1", module_path};
Frame frame1 = {module_base_address1 + 0x10, module1}; Frame frame1 = {module_base_address1 + 0x10, &module1};
const uintptr_t module_base_address2 = 0x1100; const uintptr_t module_base_address2 = 0x1100;
Module module2 = {module_base_address2, "2", module_path}; Module module2 = {module_base_address2, "2", module_path};
Frame frame2 = {module_base_address2 + 0x10, module2}; Frame frame2 = {module_base_address2 + 0x10, &module2};
std::vector<Frame> frames1 = {frame1}; std::vector<Frame> frames1 = {frame1};
std::vector<Frame> frames2 = {frame2}; std::vector<Frame> frames2 = {frame2};
...@@ -252,7 +252,7 @@ TEST(CallStackProfileBuilderTest, Modules) { ...@@ -252,7 +252,7 @@ TEST(CallStackProfileBuilderTest, Modules) {
const uintptr_t module_base_address1 = 0x1000; const uintptr_t module_base_address1 = 0x1000;
Module module1; // module1 has no information hence invalid. Module module1; // module1 has no information hence invalid.
Frame frame1 = {module_base_address1 + 0x10, module1}; Frame frame1 = {module_base_address1 + 0x10, &module1};
const uintptr_t module_base_address2 = 0x1100; const uintptr_t module_base_address2 = 0x1100;
#if defined(OS_WIN) #if defined(OS_WIN)
...@@ -263,7 +263,7 @@ TEST(CallStackProfileBuilderTest, Modules) { ...@@ -263,7 +263,7 @@ TEST(CallStackProfileBuilderTest, Modules) {
base::FilePath module_path("/some/path/to/chrome"); base::FilePath module_path("/some/path/to/chrome");
#endif #endif
Module module2 = {module_base_address2, "2", module_path}; Module module2 = {module_base_address2, "2", module_path};
Frame frame2 = {module_base_address2 + 0x10, module2}; Frame frame2 = {module_base_address2 + 0x10, &module2};
std::vector<Frame> frames = {frame1, frame2}; std::vector<Frame> frames = {frame1, frame2};
...@@ -311,11 +311,9 @@ TEST(CallStackProfileBuilderTest, DedupModules) { ...@@ -311,11 +311,9 @@ TEST(CallStackProfileBuilderTest, DedupModules) {
base::FilePath module_path("/some/path/to/chrome"); base::FilePath module_path("/some/path/to/chrome");
#endif #endif
Module module1 = {module_base_address, "1", module_path}; Module module = {module_base_address, "1", module_path};
Frame frame1 = {module_base_address + 0x10, module1}; Frame frame1 = {module_base_address + 0x10, &module};
Frame frame2 = {module_base_address + 0x20, &module};
Module module2 = {module_base_address, "1", module_path};
Frame frame2 = {module_base_address + 0x20, module2};
std::vector<Frame> frames = {frame1, frame2}; std::vector<Frame> frames = {frame1, frame2};
...@@ -334,8 +332,8 @@ TEST(CallStackProfileBuilderTest, DedupModules) { ...@@ -334,8 +332,8 @@ TEST(CallStackProfileBuilderTest, DedupModules) {
ASSERT_EQ(1, profile.stack_size()); ASSERT_EQ(1, profile.stack_size());
ASSERT_EQ(2, profile.stack(0).frame_size()); ASSERT_EQ(2, profile.stack(0).frame_size());
// Since module1 and module2 have the same base address, they are considered // The two frames share the same module, which should be deduped in the
// the same module and therefore deduped. // output.
ASSERT_TRUE(profile.stack(0).frame(0).has_module_id_index()); ASSERT_TRUE(profile.stack(0).frame(0).has_module_id_index());
EXPECT_EQ(0, profile.stack(0).frame(0).module_id_index()); EXPECT_EQ(0, profile.stack(0).frame(0).module_id_index());
ASSERT_TRUE(profile.stack(0).frame(0).has_address()); ASSERT_TRUE(profile.stack(0).frame(0).has_address());
...@@ -372,7 +370,7 @@ TEST(CallStackProfileBuilderTest, WorkIds) { ...@@ -372,7 +370,7 @@ TEST(CallStackProfileBuilderTest, WorkIds) {
#endif #endif
Module module = {0x1000, "1", module_path}; Module module = {0x1000, "1", module_path};
Frame frame = {0x1000 + 0x10, module}; Frame frame = {0x1000 + 0x10, &module};
// Id 0 means the message loop hasn't been started yet, so the sample should // Id 0 means the message loop hasn't been started yet, so the sample should
// not have continued_work set. // not have continued_work set.
...@@ -429,7 +427,7 @@ TEST(CallStackProfileBuilderTest, MetadataRecorder) { ...@@ -429,7 +427,7 @@ TEST(CallStackProfileBuilderTest, MetadataRecorder) {
#endif #endif
Module module = {0x1000, "1", module_path}; Module module = {0x1000, "1", module_path};
Frame frame = {0x1000 + 0x10, module}; Frame frame = {0x1000 + 0x10, &module};
metadata_recorder.current_value = 5; metadata_recorder.current_value = 5;
profile_builder->OnSampleCompleted({frame}); profile_builder->OnSampleCompleted({frame});
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "components/tracing/common/native_stack_sampler_android.h" #include "components/tracing/common/native_stack_sampler_android.h"
#include "base/no_destructor.h"
#include "base/sampling_heap_profiler/module_cache.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
namespace tracing { namespace tracing {
...@@ -22,6 +24,7 @@ std::vector<base::StackSamplingProfiler::Frame> ...@@ -22,6 +24,7 @@ std::vector<base::StackSamplingProfiler::Frame>
NativeStackSamplerAndroid::RecordStackFrames( NativeStackSamplerAndroid::RecordStackFrames(
StackBuffer* stack_buffer, StackBuffer* stack_buffer,
base::StackSamplingProfiler::ProfileBuilder* profile_builder) { base::StackSamplingProfiler::ProfileBuilder* profile_builder) {
static base::NoDestructor<base::ModuleCache::Module> invalid_module;
if (!unwinder_.is_initialized()) { if (!unwinder_.is_initialized()) {
// May block on disk access. This function is executed on the profiler // May block on disk access. This function is executed on the profiler
// thread, so this will only block profiling execution. // thread, so this will only block profiling execution.
...@@ -36,7 +39,7 @@ NativeStackSamplerAndroid::RecordStackFrames( ...@@ -36,7 +39,7 @@ NativeStackSamplerAndroid::RecordStackFrames(
for (size_t i = 0; i < depth; ++i) { for (size_t i = 0; i < depth; ++i) {
// TODO(ssid): Add support for obtaining modules here. // TODO(ssid): Add support for obtaining modules here.
frames.emplace_back(reinterpret_cast<uintptr_t>(pcs[i]), frames.emplace_back(reinterpret_cast<uintptr_t>(pcs[i]),
base::ModuleCache::Module()); invalid_module.get());
} }
return frames; return frames;
} }
......
...@@ -110,12 +110,12 @@ class TracingProfileBuilder ...@@ -110,12 +110,12 @@ class TracingProfileBuilder
if (frame_name.empty()) if (frame_name.empty())
frame_name = "Unknown"; frame_name = "Unknown";
#else #else
module_name = frame.module.filename.BaseName().MaybeAsASCII(); module_name = frame.module->filename.BaseName().MaybeAsASCII();
frame_name = GetFrameNameFromOffsetAddr(frame.instruction_pointer - frame_name = GetFrameNameFromOffsetAddr(frame.instruction_pointer -
frame.module.base_address); frame.module->base_address);
#endif #endif
base::StringAppendF(&result, "%s - %s [%s]\n", frame_name.c_str(), base::StringAppendF(&result, "%s - %s [%s]\n", frame_name.c_str(),
module_name.c_str(), frame.module.id.c_str()); module_name.c_str(), frame.module->id.c_str());
} }
TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("cpu_profiler"), TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("cpu_profiler"),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment