summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Srbecky <dsrbecky@google.com>2021-05-13 09:20:04 +0100
committerDavid Srbecky <dsrbecky@google.com>2021-05-13 22:11:56 +0100
commit1081fd9ad1a9ad89db2defe0582eaf561f28292a (patch)
treee1ec49d87b27f12d78745504be68ebc22bc13c9a
parent5150e1292ec6b16e4717e86b9e3cfb855eec18a3 (diff)
downloadunwinding-1081fd9ad1a9ad89db2defe0582eaf561f28292a.tar.gz
MapInfo: Lazy allocation of ELF-related fields.
MapInfos are surprisingly big in system-wide profiling. (currently taking 20% of the whole profiling process, not even counting the map name strings). This is because the number of processes times number of maps per process quickly adds up. However, many of the fields in MapInfo are related to ELF caching/loading. The number of ELF maps is relatively small compared to grand total of MapInfos. This CL therefore splits the fields into own struct, and allocates this struct on first access. This is happening transparently under the accessors. The memory use from the lazy allocated structs is negligible and this CL as good as removing the fields. The memory use from MapInfos approximately halves. Test: libunwindstack_unit_test Test: compare prefetto system-wide memory usage profiles Change-Id: I2d1f345c2c0b7d7ffb9d6f815e07e2a63939fd89
-rw-r--r--libunwindstack/Elf.cpp2
-rw-r--r--libunwindstack/MapInfo.cpp34
-rw-r--r--libunwindstack/include/unwindstack/Elf.h2
-rw-r--r--libunwindstack/include/unwindstack/MapInfo.h161
-rw-r--r--libunwindstack/tests/MapInfoTest.cpp43
5 files changed, 161 insertions, 81 deletions
diff --git a/libunwindstack/Elf.cpp b/libunwindstack/Elf.cpp
index b52d294..7f4142b 100644
--- a/libunwindstack/Elf.cpp
+++ b/libunwindstack/Elf.cpp
@@ -101,7 +101,7 @@ std::string Elf::GetSoname() {
return interface_->GetSoname();
}
-uint64_t Elf::GetRelPc(uint64_t pc, const MapInfo* map_info) {
+uint64_t Elf::GetRelPc(uint64_t pc, MapInfo* map_info) {
return pc - map_info->start() + load_bias_ + map_info->elf_offset();
}
diff --git a/libunwindstack/MapInfo.cpp b/libunwindstack/MapInfo.cpp
index 6fcfdaf..8fb6f7e 100644
--- a/libunwindstack/MapInfo.cpp
+++ b/libunwindstack/MapInfo.cpp
@@ -213,7 +213,7 @@ Memory* MapInfo::CreateMemory(const std::shared_ptr<Memory>& process_memory) {
Elf* MapInfo::GetElf(const std::shared_ptr<Memory>& process_memory, ArchEnum expected_arch) {
{
// Make sure no other thread is trying to add the elf to this map.
- std::lock_guard<std::mutex> guard(mutex_);
+ std::lock_guard<std::mutex> guard(elf_mutex());
if (elf().get() != nullptr) {
return elf().get();
@@ -259,7 +259,7 @@ Elf* MapInfo::GetElf(const std::shared_ptr<Memory>& process_memory, ArchEnum exp
// If there is a read-only map then a read-execute map that represents the
// same elf object, make sure the previous map is using the same elf
// object if it hasn't already been set.
- std::lock_guard<std::mutex> guard(prev_real_map()->mutex_);
+ std::lock_guard<std::mutex> guard(prev_real_map()->elf_mutex());
if (prev_real_map()->elf().get() == nullptr) {
prev_real_map()->set_elf(elf());
prev_real_map()->set_memory_backed_elf(memory_backed_elf());
@@ -274,7 +274,7 @@ Elf* MapInfo::GetElf(const std::shared_ptr<Memory>& process_memory, ArchEnum exp
bool MapInfo::GetFunctionName(uint64_t addr, SharedString* name, uint64_t* func_offset) {
{
// Make sure no other thread is trying to update this elf object.
- std::lock_guard<std::mutex> guard(mutex_);
+ std::lock_guard<std::mutex> guard(elf_mutex());
if (elf() == nullptr) {
return false;
}
@@ -292,7 +292,7 @@ uint64_t MapInfo::GetLoadBias(const std::shared_ptr<Memory>& process_memory) {
{
// Make sure no other thread is trying to add the elf to this map.
- std::lock_guard<std::mutex> guard(mutex_);
+ std::lock_guard<std::mutex> guard(elf_mutex());
if (elf() != nullptr) {
if (elf()->valid()) {
cur_load_bias = elf()->GetLoadBias();
@@ -314,7 +314,11 @@ uint64_t MapInfo::GetLoadBias(const std::shared_ptr<Memory>& process_memory) {
}
MapInfo::~MapInfo() {
- delete build_id().load();
+ ElfFields* elf_fields = elf_fields_.load();
+ if (elf_fields != nullptr) {
+ delete elf_fields->build_id_.load();
+ delete elf_fields;
+ }
}
SharedString MapInfo::GetBuildID() {
@@ -329,9 +333,9 @@ SharedString MapInfo::GetBuildID() {
// Now need to see if the elf object exists.
// Make sure no other thread is trying to add the elf to this map.
- mutex_.lock();
+ elf_mutex().lock();
Elf* elf_obj = elf().get();
- mutex_.unlock();
+ elf_mutex().unlock();
std::string result;
if (elf_obj != nullptr) {
result = elf_obj->GetBuildID();
@@ -360,6 +364,22 @@ SharedString MapInfo::SetBuildID(std::string&& new_build_id) {
}
}
+MapInfo::ElfFields& MapInfo::GetElfFields() {
+ ElfFields* elf_fields = elf_fields_.load(std::memory_order_acquire);
+ if (elf_fields != nullptr) {
+ return *elf_fields;
+ }
+ // Allocate and initialize the field in thread-safe way.
+ std::unique_ptr<ElfFields> desired(new ElfFields());
+ ElfFields* expected = nullptr;
+ // Strong version is reliable. Weak version might randomly return false.
+ if (elf_fields_.compare_exchange_strong(expected, desired.get())) {
+ return *desired.release(); // Success: we transferred the pointer ownership to the field.
+ } else {
+ return *expected; // Failure: 'expected' is updated to the value set by the other thread.
+ }
+}
+
std::string MapInfo::GetPrintableBuildID() {
std::string raw_build_id = GetBuildID();
if (raw_build_id.empty()) {
diff --git a/libunwindstack/include/unwindstack/Elf.h b/libunwindstack/include/unwindstack/Elf.h
index 5a496a7..01d9af9 100644
--- a/libunwindstack/include/unwindstack/Elf.h
+++ b/libunwindstack/include/unwindstack/Elf.h
@@ -57,7 +57,7 @@ class Elf {
bool GetGlobalVariableOffset(const std::string& name, uint64_t* memory_offset);
- uint64_t GetRelPc(uint64_t pc, const MapInfo* map_info);
+ uint64_t GetRelPc(uint64_t pc, MapInfo* map_info);
bool StepIfSignalHandler(uint64_t rel_pc, Regs* regs, Memory* process_memory);
diff --git a/libunwindstack/include/unwindstack/MapInfo.h b/libunwindstack/include/unwindstack/MapInfo.h
index 2a4696a..727dc68 100644
--- a/libunwindstack/include/unwindstack/MapInfo.h
+++ b/libunwindstack/include/unwindstack/MapInfo.h
@@ -31,109 +31,102 @@ namespace unwindstack {
class MemoryFileAtOffset;
+// Represents virtual memory map (as obtained from /proc/*/maps).
+//
+// Note that we have to be surprisingly careful with memory usage here,
+// since in system-wide profiling this data can take considerable space.
+// (for example, 400 process * 400 maps * 128 bytes = 20 MB + string data).
class MapInfo {
public:
MapInfo(MapInfo* prev_map, MapInfo* prev_real_map, uint64_t start, uint64_t end, uint64_t offset,
- uint64_t flags, const char* name)
- : start_(start),
- end_(end),
- offset_(offset),
- flags_(flags),
- name_(name),
- prev_map_(prev_map),
- prev_real_map_(prev_real_map),
- load_bias_(INT64_MAX),
- build_id_(0) {
- if (prev_real_map != nullptr) prev_real_map->next_real_map_ = this;
- }
- MapInfo(MapInfo* prev_map, MapInfo* prev_real_map, uint64_t start, uint64_t end, uint64_t offset,
uint64_t flags, SharedString name)
: start_(start),
end_(end),
offset_(offset),
flags_(flags),
name_(name),
+ elf_fields_(nullptr),
prev_map_(prev_map),
- prev_real_map_(prev_real_map),
- load_bias_(INT64_MAX),
- build_id_(0) {
+ prev_real_map_(prev_real_map) {
if (prev_real_map != nullptr) prev_real_map->next_real_map_ = this;
}
~MapInfo();
+ // Cached data for mapped ELF files.
+ // We allocate this structure lazily since there are much fewer ELFs than maps.
+ struct ElfFields {
+ ElfFields() : load_bias_(INT64_MAX), build_id_(0) {}
+
+ std::shared_ptr<Elf> elf_;
+ // The offset of the beginning of this mapping to the beginning of the
+ // ELF file.
+ // elf_offset == offset - elf_start_offset.
+ // This value is only non-zero if the offset is non-zero but there is
+ // no elf signature found at that offset.
+ uint64_t elf_offset_ = 0;
+ // This value is the offset into the file of the map in memory that is the
+ // start of the elf. This is not equal to offset when the linker splits
+ // shared libraries into a read-only and read-execute map.
+ uint64_t elf_start_offset_ = 0;
+
+ std::atomic_int64_t load_bias_;
+
+ // This is a pointer to a new'd std::string.
+ // Using an atomic value means that we don't need to lock and will
+ // make it easier to move to a fine grained lock in the future.
+ std::atomic<SharedString*> build_id_;
+
+ // Set to true if the elf file data is coming from memory.
+ bool memory_backed_elf_ = false;
+
+ // Protect the creation of the elf object.
+ std::mutex elf_mutex_;
+ };
+
inline uint64_t start() const { return start_; }
inline void set_start(uint64_t value) { start_ = value; }
+
inline uint64_t end() const { return end_; }
inline void set_end(uint64_t value) { end_ = value; }
+
inline uint64_t offset() const { return offset_; }
inline void set_offset(uint64_t value) { offset_ = value; }
+
inline uint16_t flags() const { return flags_; }
inline void set_flags(uint16_t value) { flags_ = value; }
+
inline SharedString& name() { return name_; }
inline void set_name(SharedString& value) { name_ = value; }
inline void set_name(const char* value) { name_ = value; }
- inline std::shared_ptr<Elf>& elf() { return elf_; }
- inline void set_elf(std::shared_ptr<Elf>& value) { elf_ = value; }
- inline void set_elf(Elf* value) { elf_.reset(value); }
- inline uint64_t elf_offset() const { return elf_offset_; }
- inline void set_elf_offset(uint64_t value) { elf_offset_ = value; }
- inline uint64_t elf_start_offset() const { return elf_start_offset_; }
- inline void set_elf_start_offset(uint64_t value) { elf_start_offset_ = value; }
- inline MapInfo* prev_map() const { return prev_map_; }
- inline void set_prev_map(MapInfo* value) { prev_map_ = value; }
- inline MapInfo* prev_real_map() const { return prev_real_map_; }
- inline void set_prev_real_map(MapInfo* value) { prev_real_map_ = value; }
- inline MapInfo* next_real_map() const { return next_real_map_; }
- inline void set_next_real_map(MapInfo* value) { next_real_map_ = value; }
- inline std::atomic_int64_t& load_bias() { return load_bias_; }
- inline void set_load_bias(int64_t value) { load_bias_ = value; }
- inline std::atomic<SharedString*>& build_id() { return build_id_; }
- inline void set_build_id(SharedString* value) { build_id_ = value; }
- inline bool memory_backed_elf() const { return memory_backed_elf_; }
- inline void set_memory_backed_elf(bool value) { memory_backed_elf_ = value; }
- private:
- uint64_t start_ = 0;
- uint64_t end_ = 0;
- uint64_t offset_ = 0;
- uint16_t flags_ = 0;
- SharedString name_;
- std::shared_ptr<Elf> elf_;
- // The offset of the beginning of this mapping to the beginning of the
- // ELF file.
- // elf_offset == offset - elf_start_offset.
- // This value is only non-zero if the offset is non-zero but there is
- // no elf signature found at that offset.
- uint64_t elf_offset_ = 0;
- // This value is the offset into the file of the map in memory that is the
- // start of the elf. This is not equal to offset when the linker splits
- // shared libraries into a read-only and read-execute map.
- uint64_t elf_start_offset_ = 0;
+ inline std::shared_ptr<Elf>& elf() { return GetElfFields().elf_; }
+ inline void set_elf(std::shared_ptr<Elf>& value) { GetElfFields().elf_ = value; }
+ inline void set_elf(Elf* value) { GetElfFields().elf_.reset(value); }
- MapInfo* prev_map_ = nullptr;
- // This is the previous map that is not empty with a 0 offset. For
- // example, this set of maps:
- // 1000-2000 r--p 000000 00:00 0 libc.so
- // 2000-3000 ---p 000000 00:00 0 libc.so
- // 3000-4000 r-xp 003000 00:00 0 libc.so
- // The last map's prev_map would point to the 2000-3000 map, while the
- // prev_real_map would point to the 1000-2000 map.
- MapInfo* prev_real_map_ = nullptr;
+ inline uint64_t elf_offset() { return GetElfFields().elf_offset_; }
+ inline void set_elf_offset(uint64_t value) { GetElfFields().elf_offset_ = value; }
- // Same as above but set to point to the next map.
- MapInfo* next_real_map_ = nullptr;
+ inline uint64_t elf_start_offset() { return GetElfFields().elf_start_offset_; }
+ inline void set_elf_start_offset(uint64_t value) { GetElfFields().elf_start_offset_ = value; }
- std::atomic_int64_t load_bias_;
+ inline std::atomic_int64_t& load_bias() { return GetElfFields().load_bias_; }
+ inline void set_load_bias(int64_t value) { GetElfFields().load_bias_ = value; }
- // This is a pointer to a new'd std::string.
- // Using an atomic value means that we don't need to lock and will
- // make it easier to move to a fine grained lock in the future.
- std::atomic<SharedString*> build_id_;
+ inline std::atomic<SharedString*>& build_id() { return GetElfFields().build_id_; }
+ inline void set_build_id(SharedString* value) { GetElfFields().build_id_ = value; }
- // Set to true if the elf file data is coming from memory.
- bool memory_backed_elf_ = false;
+ inline bool memory_backed_elf() { return GetElfFields().memory_backed_elf_; }
+ inline void set_memory_backed_elf(bool value) { GetElfFields().memory_backed_elf_ = value; }
+
+ inline MapInfo* prev_map() const { return prev_map_; }
+ inline void set_prev_map(MapInfo* value) { prev_map_ = value; }
+
+ inline MapInfo* prev_real_map() const { return prev_real_map_; }
+ inline void set_prev_real_map(MapInfo* value) { prev_real_map_ = value; }
+
+ inline MapInfo* next_real_map() const { return next_real_map_; }
+ inline void set_next_real_map(MapInfo* value) { next_real_map_ = value; }
- public:
// This function guarantees it will never return nullptr.
Elf* GetElf(const std::shared_ptr<Memory>& process_memory, ArchEnum expected_arch);
@@ -154,6 +147,9 @@ class MapInfo {
inline bool IsBlank() { return offset() == 0 && flags() == 0 && name().empty(); }
+ // Returns elf_fields_. It will create the object if it is null.
+ ElfFields& GetElfFields();
+
private:
MapInfo(const MapInfo&) = delete;
void operator=(const MapInfo&) = delete;
@@ -162,7 +158,28 @@ class MapInfo {
bool InitFileMemoryFromPreviousReadOnlyMap(MemoryFileAtOffset* memory);
// Protect the creation of the elf object.
- std::mutex mutex_;
+ std::mutex& elf_mutex() { return GetElfFields().elf_mutex_; }
+
+ uint64_t start_ = 0;
+ uint64_t end_ = 0;
+ uint64_t offset_ = 0;
+ uint16_t flags_ = 0;
+ SharedString name_;
+
+ std::atomic<ElfFields*> elf_fields_;
+
+ MapInfo* prev_map_ = nullptr;
+ // This is the previous map that is not empty with a 0 offset. For
+ // example, this set of maps:
+ // 1000-2000 r--p 000000 00:00 0 libc.so
+ // 2000-3000 ---p 000000 00:00 0 libc.so
+ // 3000-4000 r-xp 003000 00:00 0 libc.so
+ // The last map's prev_map would point to the 2000-3000 map, while the
+ // prev_real_map would point to the 1000-2000 map.
+ MapInfo* prev_real_map_ = nullptr;
+
+ // Same as above but set to point to the next map.
+ MapInfo* next_real_map_ = nullptr;
};
} // namespace unwindstack
diff --git a/libunwindstack/tests/MapInfoTest.cpp b/libunwindstack/tests/MapInfoTest.cpp
index 59aa79e..055e965 100644
--- a/libunwindstack/tests/MapInfoTest.cpp
+++ b/libunwindstack/tests/MapInfoTest.cpp
@@ -16,6 +16,8 @@
#include <stdint.h>
+#include <thread>
+
#include <gtest/gtest.h>
#include <unwindstack/MapInfo.h>
@@ -72,4 +74,45 @@ TEST(MapInfoTest, get_function_name) {
EXPECT_EQ(1000UL, offset);
}
+TEST(MapInfoTest, multiple_thread_get_elf_fields) {
+ MapInfo map_info(nullptr, nullptr, 0, 0, 0, 0, "");
+
+ static constexpr size_t kNumConcurrentThreads = 100;
+ MapInfo::ElfFields* fields[kNumConcurrentThreads];
+ Elf* elfs[kNumConcurrentThreads];
+ uint64_t offsets[kNumConcurrentThreads];
+
+ std::atomic_bool wait;
+ wait = true;
+ // Create all of the threads and have them do the call at the same time
+ // to make it likely that a race will occur.
+ std::vector<std::thread*> threads;
+ for (size_t i = 0; i < kNumConcurrentThreads; i++) {
+ std::thread* thread = new std::thread([&]() {
+ while (wait)
+ ;
+ fields[i] = &map_info.GetElfFields();
+ elfs[i] = fields[i]->elf_.get();
+ offsets[i] = fields[i]->elf_offset_;
+ });
+ threads.push_back(thread);
+ }
+
+ // Set them all going and wait for the threads to finish.
+ wait = false;
+ for (auto thread : threads) {
+ thread->join();
+ delete thread;
+ }
+
+ // Now verify that all of them are exactly the same and valid.
+ for (size_t i = 0; i < kNumConcurrentThreads; i++) {
+ EXPECT_EQ(fields[0], fields[i]) << "Thread " << i << " mismatched.";
+ EXPECT_EQ(elfs[0], elfs[i]) << "Thread " << i << " mismatched.";
+ EXPECT_EQ(offsets[0], offsets[i]) << "Thread " << i << " mismatched.";
+ EXPECT_EQ(nullptr, elfs[i]);
+ EXPECT_EQ(0u, offsets[i]);
+ }
+}
+
} // namespace unwindstack