Merge "trace_processor: reduce memory + ingestion time for utid map"
diff --git a/Android.bp b/Android.bp
index a22d077..edd36be 100644
--- a/Android.bp
+++ b/Android.bp
@@ -169,7 +169,6 @@
],
shared_libs: [
"libbase",
- "liblog",
"libprocinfo",
"libunwindstack",
],
@@ -2383,7 +2382,6 @@
"src/tracing/ipc/service/service_ipc_host_impl.cc",
],
shared_libs: [
- "liblog",
"libprotobuf-cpp-lite",
],
static_libs: [
@@ -2458,7 +2456,6 @@
":perfetto_protos_perfetto_trace_sys_stats_lite_gen",
],
shared_libs: [
- "liblog",
"libprotobuf-cpp-lite",
],
host_supported: true,
diff --git a/BUILD.gn b/BUILD.gn
index 2b60803..2d3a59b 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -295,6 +295,7 @@
if (perfetto_build_with_android) {
# TODO(fmayer): Investigate shared library for common pieces.
shared_library("heapprofd_client") {
+ configs -= [ "//gn/standalone:android_liblog" ]
cflags = [ "-DPERFETTO_ANDROID_ASYNC_SAFE_LOG" ]
deps = [
"src/profiling/memory:malloc_hooks",
diff --git a/gn/standalone/BUILD.gn b/gn/standalone/BUILD.gn
index e7aabf0..ee17fff 100644
--- a/gn/standalone/BUILD.gn
+++ b/gn/standalone/BUILD.gn
@@ -120,10 +120,6 @@
]
}
- if (is_android) {
- libs += [ "log" ]
- }
-
if (is_debug) {
libs += [ "dl" ]
}
@@ -241,3 +237,13 @@
]
}
}
+
+# This config is only added to certain leaf target types (see BUILDCONFIG.gn).
+# This allows us to remove the config (and thus the dependency) on a per-target
+# basis. If the config was applied to source_sets, then they would unavoidably
+# carry the dependency onto liblog to the final target.
+config("android_liblog") {
+ if (is_android) {
+ libs = [ "log" ]
+ }
+}
diff --git a/gn/standalone/BUILDCONFIG.gn b/gn/standalone/BUILDCONFIG.gn
index cc2f1b8..e1bc1c2 100644
--- a/gn/standalone/BUILDCONFIG.gn
+++ b/gn/standalone/BUILDCONFIG.gn
@@ -86,11 +86,17 @@
set_defaults("shared_library") {
configs = default_configs
configs += [ "//gn/standalone:shared_library" ]
+
+ # note: the following is not a nested config to be removable.
+ configs += [ "//gn/standalone:android_liblog" ]
}
set_defaults("executable") {
configs = default_configs
configs += [ "//gn/standalone:executable" ]
+
+ # note: the following is not a nested config to be removable.
+ configs += [ "//gn/standalone:android_liblog" ]
}
set_default_toolchain("//gn/standalone/toolchain:gcc_like")
diff --git a/src/profiling/memory/bookkeeping.cc b/src/profiling/memory/bookkeeping.cc
index cf481c6..999f3de 100644
--- a/src/profiling/memory/bookkeeping.cc
+++ b/src/profiling/memory/bookkeeping.cc
@@ -361,8 +361,18 @@
trace_writer->Flush(dump_rec.callback);
} else if (rec->record_type == BookkeepingRecord::Type::Free) {
FreeRecord& free_rec = rec->free_record;
- FreePageEntry* entries = free_rec.metadata->entries;
- uint64_t num_entries = free_rec.metadata->num_entries;
+ FreeMetadata& free_metadata = *free_rec.metadata;
+
+ if (bookkeeping_data->client_generation < free_metadata.client_generation) {
+ bookkeeping_data->heap_tracker = HeapTracker(&callsites_);
+ bookkeeping_data->client_generation = free_metadata.client_generation;
+ } else if (bookkeeping_data->client_generation >
+ free_metadata.client_generation) {
+ return;
+ }
+
+ FreePageEntry* entries = free_metadata.entries;
+ uint64_t num_entries = free_metadata.num_entries;
if (num_entries > kFreePageSize)
return;
for (size_t i = 0; i < num_entries; ++i) {
@@ -372,6 +382,17 @@
}
} else if (rec->record_type == BookkeepingRecord::Type::Malloc) {
AllocRecord& alloc_rec = rec->alloc_record;
+ AllocMetadata& alloc_metadata = alloc_rec.alloc_metadata;
+
+ if (bookkeeping_data->client_generation <
+ alloc_metadata.client_generation) {
+ bookkeeping_data->heap_tracker = HeapTracker(&callsites_);
+ bookkeeping_data->client_generation = alloc_metadata.client_generation;
+ } else if (bookkeeping_data->client_generation >
+ alloc_metadata.client_generation) {
+ return;
+ }
+
bookkeeping_data->heap_tracker.RecordMalloc(
alloc_rec.frames, alloc_rec.alloc_metadata.alloc_address,
alloc_rec.alloc_metadata.total_size,
diff --git a/src/profiling/memory/bookkeeping.h b/src/profiling/memory/bookkeeping.h
index 4d26fe7..21b365c 100644
--- a/src/profiling/memory/bookkeeping.h
+++ b/src/profiling/memory/bookkeeping.h
@@ -357,7 +357,7 @@
// The sequence number all mallocs and frees have been handled up to.
uint64_t committed_sequence_number_ = 0;
- GlobalCallstackTrie* const callsites_;
+ GlobalCallstackTrie* callsites_;
};
struct BookkeepingData {
@@ -366,12 +366,12 @@
explicit BookkeepingData(GlobalCallstackTrie* callsites)
: heap_tracker(callsites) {}
- HeapTracker heap_tracker;
-
// This is different to a shared_ptr to HeapTracker, because we want to keep
// it around until the first dump after the last socket for the PID has
- // disconnected.
+ // disconnected
uint64_t ref_count = 0;
+ uint64_t client_generation = 0;
+ HeapTracker heap_tracker;
};
// BookkeepingThread owns the BookkeepingData for all processes. The Run()
diff --git a/src/profiling/memory/client.cc b/src/profiling/memory/client.cc
index 38f50be..807b091 100644
--- a/src/profiling/memory/client.cc
+++ b/src/profiling/memory/client.cc
@@ -200,9 +200,13 @@
return stackaddr + stacksize;
}
+std::atomic<uint64_t> Client::max_generation_{0};
+
Client::Client(std::vector<base::UnixSocketRaw> socks)
- : pthread_key_(ThreadLocalSamplingData::KeyDestructor),
+ : generation_(++max_generation_),
+ pthread_key_(ThreadLocalSamplingData::KeyDestructor),
socket_pool_(std::move(socks)),
+ free_page_(generation_),
main_thread_stack_base_(FindMainThreadStack()) {
PERFETTO_DCHECK(pthread_key_.valid());
@@ -300,6 +304,7 @@
}
uint64_t stack_size = static_cast<uint64_t>(stackbase - stacktop);
+ metadata.client_generation = generation_;
metadata.total_size = total_size;
metadata.alloc_size = alloc_size;
metadata.alloc_address = alloc_address;
diff --git a/src/profiling/memory/client.h b/src/profiling/memory/client.h
index b567f27..ead3e1b 100644
--- a/src/profiling/memory/client.h
+++ b/src/profiling/memory/client.h
@@ -83,6 +83,10 @@
// free separately, so we batch and send the whole buffer once it is full.
class FreePage {
public:
+ FreePage(uint64_t client_generation) {
+ free_page_.client_generation = client_generation;
+ }
+
// Add address to buffer. Flush if necessary using a socket borrowed from
// pool.
// Can be called from any thread. Must not hold mutex_.`
@@ -149,6 +153,9 @@
void (*unhooked_free)(void*));
const char* GetStackBase();
+ static std::atomic<uint64_t> max_generation_;
+ const uint64_t generation_;
+
std::atomic<bool> inited_{false};
ClientConfiguration client_config_;
PThreadKey pthread_key_;
diff --git a/src/profiling/memory/client_unittest.cc b/src/profiling/memory/client_unittest.cc
index 62185aa..cb794b0 100644
--- a/src/profiling/memory/client_unittest.cc
+++ b/src/profiling/memory/client_unittest.cc
@@ -118,7 +118,7 @@
socks.emplace_back(CreateSocket());
SocketPool pool(std::move(socks));
pool.Shutdown();
- FreePage p;
+ FreePage p{0};
p.Add(0, 1, &pool);
}
diff --git a/src/profiling/memory/heapprofd_end_to_end_test.cc b/src/profiling/memory/heapprofd_end_to_end_test.cc
index 9789d25..67e7f17 100644
--- a/src/profiling/memory/heapprofd_end_to_end_test.cc
+++ b/src/profiling/memory/heapprofd_end_to_end_test.cc
@@ -46,6 +46,8 @@
namespace profiling {
namespace {
+constexpr useconds_t kMsToUs = 1000;
+
using ::testing::Eq;
using ::testing::AnyOf;
@@ -119,6 +121,7 @@
x[1] = 'x';
free(const_cast<char*>(x));
}
+ usleep(10 * kMsToUs);
}
default:
break;
@@ -326,7 +329,7 @@
}
// TODO(fmayer): Enable in CL that fixes b/123352823.
-TEST_F(HeapprofdEndToEnd, DISABLED_ReInit) {
+TEST_F(HeapprofdEndToEnd, ReInit) {
constexpr uint64_t kFirstIterationBytes = 5;
constexpr uint64_t kSecondIterationBytes = 7;
@@ -356,6 +359,7 @@
signal_pipe.rd.reset();
ack_pipe.wr.reset();
}
+ usleep(10 * kMsToUs);
}
PERFETTO_FATAL("Should be unreachable");
}
diff --git a/src/profiling/memory/queue_messages.h b/src/profiling/memory/queue_messages.h
index 04a42c5..be3e845 100644
--- a/src/profiling/memory/queue_messages.h
+++ b/src/profiling/memory/queue_messages.h
@@ -70,6 +70,7 @@
Free = 2,
};
pid_t pid;
+ uint64_t client_generation;
// TODO(fmayer): Use a union.
Type record_type;
AllocRecord alloc_record;
diff --git a/src/profiling/memory/unwinding.cc b/src/profiling/memory/unwinding.cc
index 4a74d1f..43d55d1 100644
--- a/src/profiling/memory/unwinding.cc
+++ b/src/profiling/memory/unwinding.cc
@@ -206,7 +206,8 @@
if (error_code != unwindstack::ERROR_INVALID_MAP)
break;
}
- for (unwindstack::FrameData fd : unwinder.frames()) {
+ std::vector<unwindstack::FrameData> frames = unwinder.ConsumeFrames();
+ for (unwindstack::FrameData& fd : frames) {
std::string build_id;
if (fd.map_name != "") {
unwindstack::MapInfo* map_info = metadata->maps.Find(fd.pc);
@@ -245,12 +246,14 @@
out->alloc_record.alloc_metadata = *msg.alloc_header;
out->pid = rec->pid;
+ out->client_generation = msg.alloc_header->client_generation;
out->record_type = BookkeepingRecord::Type::Malloc;
DoUnwind(&msg, metadata.get(), &out->alloc_record);
return true;
} else if (msg.record_type == RecordType::Free) {
out->record_type = BookkeepingRecord::Type::Free;
out->pid = rec->pid;
+ out->client_generation = msg.free_header->client_generation;
// We need to keep this alive, because msg.free_header is a pointer into
// this.
out->free_record.free_data = std::move(rec->data);
diff --git a/src/profiling/memory/wire_protocol.h b/src/profiling/memory/wire_protocol.h
index 00db8e1..a6fde3e 100644
--- a/src/profiling/memory/wire_protocol.h
+++ b/src/profiling/memory/wire_protocol.h
@@ -76,6 +76,7 @@
};
struct AllocMetadata {
+ uint64_t client_generation;
uint64_t sequence_number;
// Size of the allocation that was made.
uint64_t alloc_size;
@@ -106,6 +107,8 @@
};
struct FreeMetadata {
+ uint64_t client_generation;
+
uint64_t num_entries;
FreePageEntry entries[kFreePageSize];
};
diff --git a/tools/heap_profile b/tools/heap_profile
index 432e3d0..aeb266f 100755
--- a/tools/heap_profile
+++ b/tools/heap_profile
@@ -30,8 +30,8 @@
import urllib
TRACE_TO_TEXT_SHAS = {
- 'linux': '02b80116b5873dc00d82ff50c79f89909e054e78',
- 'mac': 'f0fcf2e9adaacd28c1b712b241d5f02e9f746741',
+ 'linux': '4e52b07b2f6258643d16610ae1aae8ac73063624',
+ 'mac': '0893f2892f519c6c48458bbc34a61dbed50c20a5',
}
TRACE_TO_TEXT_PATH = tempfile.gettempdir()
TRACE_TO_TEXT_BASE_URL = (
diff --git a/tools/install-build-deps b/tools/install-build-deps
index 2b79343..cde25f1 100755
--- a/tools/install-build-deps
+++ b/tools/install-build-deps
@@ -170,7 +170,7 @@
# These dependencies are for libunwindstack, which is used by src/profiling.
('buildtools/android-core',
'https://android.googlesource.com/platform/system/core.git',
- 'ad6a5c565c4908e3ae292f26510a2cc3c8bdf046',
+ 'dd70df2e69d06abd87c00b957840248397917dcc',
'all'
),
diff --git a/tools/trace_to_text/trace_to_profile.cc b/tools/trace_to_text/trace_to_profile.cc
index 6b07312..b8e79b7 100644
--- a/tools/trace_to_text/trace_to_profile.cc
+++ b/tools/trace_to_text/trace_to_profile.cc
@@ -68,6 +68,16 @@
return hex_build_id;
}
+enum Strings : int64_t {
+ kEmpty = 0,
+ kObjects,
+ kAllocObjects,
+ kCount,
+ kSpace,
+ kAllocSpace,
+ kBytes
+};
+
void DumpProfilePacket(std::vector<ProfilePacket>& packet_fragments,
const std::string& file_prefix) {
std::map<uint64_t, std::string> string_lookup;
@@ -91,15 +101,31 @@
}
std::map<std::string, uint64_t> string_table;
- string_table[""] = 0;
- string_table["space"] = 1;
- string_table["bytes"] = 2;
+ string_table[""] = kEmpty;
+ string_table["objects"] = kObjects;
+ string_table["alloc_objects"] = kAllocObjects;
+ string_table["count"] = kCount;
+ string_table["space"] = kSpace;
+ string_table["alloc_space"] = kAllocSpace;
+ string_table["bytes"] = kBytes;
GProfile profile;
GValueType* value_type = profile.add_sample_type();
- // ["space", "bytes"];
- value_type->set_type(1);
- value_type->set_type(2);
+ value_type->set_type(kObjects);
+ value_type->set_unit(kCount);
+
+ value_type = profile.add_sample_type();
+ value_type->set_type(kAllocObjects);
+ value_type->set_unit(kCount);
+
+ value_type = profile.add_sample_type();
+ value_type->set_type(kAllocSpace);
+ value_type->set_unit(kBytes);
+
+ // The last value is the default one selected.
+ value_type = profile.add_sample_type();
+ value_type->set_type(kSpace);
+ value_type->set_unit(kBytes);
for (const ProfilePacket& packet : packet_fragments) {
for (const ProfilePacket::Mapping& mapping : packet.mappings()) {
@@ -196,6 +222,10 @@
}
for (uint64_t frame_id : it->second)
gsample->add_location_id(frame_id);
+ gsample->add_value(
+ static_cast<int64_t>(sample.alloc_count() - sample.free_count()));
+ gsample->add_value(static_cast<int64_t>(sample.alloc_count()));
+ gsample->add_value(static_cast<int64_t>(sample.self_allocated()));
gsample->add_value(static_cast<int64_t>(sample.self_allocated() -
sample.self_freed()));
}
diff --git a/ui/src/controller/trace_controller.ts b/ui/src/controller/trace_controller.ts
index 6c0d409..5ffbffb 100644
--- a/ui/src/controller/trace_controller.ts
+++ b/ui/src/controller/trace_controller.ts
@@ -259,7 +259,7 @@
trackGroup: SCROLLING_TRACK_GROUP,
config: {
cpu,
- maximumValue: maxFreq.columns[0].longValues![0],
+ maximumValue: +maxFreq.columns[0].doubleValues![0],
}
}));
}
diff --git a/ui/src/tracks/cpu_freq/frontend.ts b/ui/src/tracks/cpu_freq/frontend.ts
index c7eac0e..894bc12 100644
--- a/ui/src/tracks/cpu_freq/frontend.ts
+++ b/ui/src/tracks/cpu_freq/frontend.ts
@@ -189,7 +189,8 @@
ctx.fillText(text, this.mouseXpos + 10, centerY - 3);
// Display idle value if current hover is idle.
if (this.hoveredIdle !== undefined && this.hoveredIdle !== -1) {
- const idle = `idle: ${this.hoveredIdle.toLocaleString()}`;
+ // Display the idle value +1 to be consistent with catapult.
+ const idle = `idle: ${(this.hoveredIdle + 1).toLocaleString()}`;
ctx.fillText(idle, this.mouseXpos + 10, centerY + 11);
}
}