Add support for thread names to out of process heap profiling.
This CL adds a new memlog stack mode: native-with-thread-names. When enabled,
the thread name will be inserted as the first frame of each stack.
Other minor changes:
* Fixed minor bug in the parsing logic for ProfilingTestDriver.
* Fixed a JSON exporter issue [node and string ids should begin at 1, not 0.
chrome://tracing UI ignores nodes with id 0.]
* Add TLS destructor for allocator shim.
* base::android::AttachCurrentThread() now preserves thread name.
Change-Id: I8c9d82084d6439e663f94e563068c987d1cf3b23
Bug: 758739
Reviewed-on: https://chromium-review.googlesource.com/877085
Reviewed-by: Maria Khomenko <[email protected]>
Reviewed-by: Richard Coles <[email protected]>
Reviewed-by: Dmitry Skiba <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Reviewed-by: Albert J. Wong <[email protected]>
Commit-Queue: Erik Chen <[email protected]>
Cr-Commit-Position: refs/heads/master@{#531992}diff --git a/base/android/jni_android.cc b/base/android/jni_android.cc
index 71bd2d3..d4e8d9e 100644
--- a/base/android/jni_android.cc
+++ b/base/android/jni_android.cc
@@ -5,6 +5,7 @@
#include "base/android/jni_android.h"
#include <stddef.h>
+#include <sys/prctl.h>
#include <map>
@@ -38,9 +39,26 @@
JNIEnv* AttachCurrentThread() {
DCHECK(g_jvm);
- JNIEnv* env = NULL;
- jint ret = g_jvm->AttachCurrentThread(&env, NULL);
- DCHECK_EQ(JNI_OK, ret);
+ JNIEnv* env = nullptr;
+ jint ret = g_jvm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_2);
+ if (ret == JNI_EDETACHED || !env) {
+ JavaVMAttachArgs args;
+ args.version = JNI_VERSION_1_2;
+ args.group = nullptr;
+
+ // 16 is the maximum size for thread names on Android.
+ char thread_name[16];
+ int err = prctl(PR_GET_NAME, thread_name);
+ if (err < 0) {
+ DPLOG(ERROR) << "prctl(PR_GET_NAME)";
+ args.name = nullptr;
+ } else {
+ args.name = thread_name;
+ }
+
+ ret = g_jvm->AttachCurrentThread(&env, &args);
+ DCHECK_EQ(JNI_OK, ret);
+ }
return env;
}
diff --git a/base/threading/thread_id_name_manager.cc b/base/threading/thread_id_name_manager.cc
index 74a42c7d..ebcc3ce 100644
--- a/base/threading/thread_id_name_manager.cc
+++ b/base/threading/thread_id_name_manager.cc
@@ -47,6 +47,11 @@
name_to_interned_name_[kDefaultName];
}
+void ThreadIdNameManager::InstallSetNameCallback(SetNameCallback callback) {
+ AutoLock locked(lock_);
+ set_name_callback_ = std::move(callback);
+}
+
void ThreadIdNameManager::SetName(PlatformThreadId id,
const std::string& name) {
std::string* leaked_str = nullptr;
@@ -63,6 +68,10 @@
ThreadIdToHandleMap::iterator id_to_handle_iter =
thread_id_to_handle_.find(id);
+ if (set_name_callback_) {
+ set_name_callback_.Run(leaked_str->c_str());
+ }
+
// The main thread of a process will not be created as a Thread object which
// means there is no PlatformThreadHandler registered.
if (id_to_handle_iter == thread_id_to_handle_.end()) {
diff --git a/base/threading/thread_id_name_manager.h b/base/threading/thread_id_name_manager.h
index f469b60..d0717b09 100644
--- a/base/threading/thread_id_name_manager.h
+++ b/base/threading/thread_id_name_manager.h
@@ -9,6 +9,7 @@
#include <string>
#include "base/base_export.h"
+#include "base/callback.h"
#include "base/macros.h"
#include "base/synchronization/lock.h"
#include "base/threading/platform_thread.h"
@@ -27,6 +28,12 @@
// Register the mapping between a thread |id| and |handle|.
void RegisterThread(PlatformThreadHandle::Handle handle, PlatformThreadId id);
+ // The callback is called on the thread, immediately after the name is set.
+ // |name| is a pointer to a C string that is guaranteed to remain valid for
+ // the duration of the process.
+ using SetNameCallback = base::RepeatingCallback<void(const char* name)>;
+ void InstallSetNameCallback(SetNameCallback callback);
+
// Set the name for the given id.
void SetName(PlatformThreadId id, const std::string& name);
@@ -60,6 +67,8 @@
std::string* main_process_name_;
PlatformThreadId main_process_id_;
+ SetNameCallback set_name_callback_;
+
DISALLOW_COPY_AND_ASSIGN(ThreadIdNameManager);
};
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java
index 32f9ff1b..0baacdf 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java
@@ -37,24 +37,25 @@
@Test
@MediumTest
- @CommandLineFlags.Add({"memlog=browser"})
+ @CommandLineFlags.Add({"memlog=browser", "memlog-stack-mode=native-include-thread-names"})
public void testModeBrowser() throws Exception {
TestAndroidShim profilingProcessHost = new TestAndroidShim();
- Assert.assertTrue(profilingProcessHost.runTestForMode("browser", false, false));
+ Assert.assertTrue(profilingProcessHost.runTestForMode(
+ "browser", false, "native-include-thread-names"));
}
@Test
@MediumTest
public void testModeBrowserDynamic() throws Exception {
TestAndroidShim profilingProcessHost = new TestAndroidShim();
- Assert.assertTrue(profilingProcessHost.runTestForMode("browser", true, false));
+ Assert.assertTrue(profilingProcessHost.runTestForMode("browser", true, "native"));
}
@Test
@MediumTest
public void testModeBrowserDynamicPseudo() throws Exception {
TestAndroidShim profilingProcessHost = new TestAndroidShim();
- Assert.assertTrue(profilingProcessHost.runTestForMode("browser", true, true));
+ Assert.assertTrue(profilingProcessHost.runTestForMode("browser", true, "pseudo"));
}
// Non-browser processes must be profiled with a command line flag, since
@@ -67,7 +68,7 @@
@DisabledTest
public void testModeRendererPseudo() throws Exception {
TestAndroidShim profilingProcessHost = new TestAndroidShim();
- Assert.assertTrue(profilingProcessHost.runTestForMode("all-renderers", false, true));
+ Assert.assertTrue(profilingProcessHost.runTestForMode("all-renderers", false, "pseudo"));
}
@Test
@@ -75,6 +76,6 @@
@CommandLineFlags.Add({"memlog=gpu", "memlog-stack-mode=pseudo"})
public void testModeGpuPseudo() throws Exception {
TestAndroidShim profilingProcessHost = new TestAndroidShim();
- Assert.assertTrue(profilingProcessHost.runTestForMode("gpu", false, true));
+ Assert.assertTrue(profilingProcessHost.runTestForMode("gpu", false, "native"));
}
}
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/TestAndroidShim.java b/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/TestAndroidShim.java
index 392d98c..7a94161 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/TestAndroidShim.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/TestAndroidShim.java
@@ -23,9 +23,9 @@
* rather than native stacks.
*/
public boolean runTestForMode(
- String mode, boolean dynamicallyStartProfiling, boolean pseudoStacks) {
+ String mode, boolean dynamicallyStartProfiling, String stackMode) {
return nativeRunTestForMode(
- mNativeTestAndroidShim, mode, dynamicallyStartProfiling, pseudoStacks);
+ mNativeTestAndroidShim, mode, dynamicallyStartProfiling, stackMode);
}
/**
@@ -43,5 +43,5 @@
private native long nativeInit();
private native void nativeDestroy(long nativeTestAndroidShim);
private native boolean nativeRunTestForMode(long nativeTestAndroidShim, String mode,
- boolean dynamicallyStartProfiling, boolean pseudoStacks);
+ boolean dynamicallyStartProfiling, String stackMode);
}
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index bfa0c62f..5ed7048 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -991,6 +991,9 @@
{flags_ui::kGenericExperimentChoiceDisabled, "", ""},
{flag_descriptions::kOOPHPStackModeNative, switches::kMemlogStackMode,
switches::kMemlogStackModeNative},
+ {flag_descriptions::kOOPHPStackModeNativeWithThreadNames,
+ switches::kMemlogStackMode,
+ switches::kMemlogStackModeNativeWithThreadNames},
{flag_descriptions::kOOPHPStackModePseudo, switches::kMemlogStackMode,
switches::kMemlogStackModePseudo},
{flag_descriptions::kOOPHPStackModeMixed, switches::kMemlogStackMode,
diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
index 7fb909f..ea92168 100644
--- a/chrome/browser/flag_descriptions.cc
+++ b/chrome/browser/flag_descriptions.cc
@@ -532,11 +532,13 @@
const char kOOPHPStackModeDescription[] =
"By default, memlog heap dumps record native stacks, which requires a "
"post-processing step to symbolize. Requires a custom build with frame "
- "pointers to work on Android. It's also possible to record a pseudo stack "
- "using trace events as identifiers. It's also possible to do a mix of "
- "both.";
+ "pointers to work on Android. Native with thread names will add the thread "
+ "name as the first frame of each native stack. It's also possible to "
+ "record a pseudo stack using trace events as identifiers. It's also "
+ "possible to do a mix of both.";
const char kOOPHPStackModeMixed[] = "Mixed";
const char kOOPHPStackModeNative[] = "Native";
+const char kOOPHPStackModeNativeWithThreadNames[] = "Native with thread names";
const char kOOPHPStackModePseudo[] = "Trace events";
const char kEnablePictureInPictureName[] = "Enable picture in picture.";
diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
index 30c7af5..9209b4ad 100644
--- a/chrome/browser/flag_descriptions.h
+++ b/chrome/browser/flag_descriptions.h
@@ -339,6 +339,7 @@
extern const char kOOPHPStackModeDescription[];
extern const char kOOPHPStackModeMixed[];
extern const char kOOPHPStackModeNative[];
+extern const char kOOPHPStackModeNativeWithThreadNames[];
extern const char kOOPHPStackModePseudo[];
extern const char kEnablePictureInPictureName[];
diff --git a/chrome/browser/profiling_host/memlog_browsertest.cc b/chrome/browser/profiling_host/memlog_browsertest.cc
index 623ea3f..7deb542 100644
--- a/chrome/browser/profiling_host/memlog_browsertest.cc
+++ b/chrome/browser/profiling_host/memlog_browsertest.cc
@@ -54,7 +54,13 @@
if (GetParam().stack_mode == mojom::StackMode::PSEUDO) {
command_line->AppendSwitchASCII(switches::kMemlogStackMode,
switches::kMemlogStackModePseudo);
- } else if (GetParam().stack_mode == mojom::StackMode::NATIVE) {
+ } else if (GetParam().stack_mode ==
+ mojom::StackMode::NATIVE_WITH_THREAD_NAMES) {
+ command_line->AppendSwitchASCII(
+ switches::kMemlogStackMode,
+ switches::kMemlogStackModeNativeWithThreadNames);
+ } else if (GetParam().stack_mode ==
+ mojom::StackMode::NATIVE_WITHOUT_THREAD_NAMES) {
command_line->AppendSwitchASCII(switches::kMemlogStackMode,
switches::kMemlogStackModeNative);
} else if (GetParam().stack_mode == mojom::StackMode::MIXED) {
@@ -97,7 +103,7 @@
std::vector<mojom::StackMode> stack_modes;
stack_modes.push_back(mojom::StackMode::MIXED);
- stack_modes.push_back(mojom::StackMode::NATIVE);
+ stack_modes.push_back(mojom::StackMode::NATIVE_WITHOUT_THREAD_NAMES);
stack_modes.push_back(mojom::StackMode::PSEUDO);
for (const auto& mode : dynamic_start_modes) {
@@ -117,6 +123,10 @@
params.push_back({mode, stack_mode, true});
}
}
+
+ // Test thread names for native profiling.
+ params.push_back({ProfilingProcessHost::Mode::kBrowser,
+ mojom::StackMode::NATIVE_WITH_THREAD_NAMES, false});
return params;
}
diff --git a/chrome/browser/profiling_host/profiling_process_host.cc b/chrome/browser/profiling_host/profiling_process_host.cc
index 3b2fdb4..e21ab4263 100644
--- a/chrome/browser/profiling_host/profiling_process_host.cc
+++ b/chrome/browser/profiling_host/profiling_process_host.cc
@@ -431,20 +431,30 @@
kOOPHeapProfilingFeature, kOOPHeapProfilingFeatureStackMode);
}
- if (stack_mode == switches::kMemlogStackModeNative)
- return profiling::mojom::StackMode::NATIVE;
- if (stack_mode == switches::kMemlogStackModePseudo)
+ return ConvertStringToStackMode(stack_mode);
+}
+
+// static
+mojom::StackMode ProfilingProcessHost::ConvertStringToStackMode(
+ const std::string& input) {
+ if (input == switches::kMemlogStackModeNative)
+ return profiling::mojom::StackMode::NATIVE_WITHOUT_THREAD_NAMES;
+ if (input == switches::kMemlogStackModeNativeWithThreadNames)
+ return profiling::mojom::StackMode::NATIVE_WITH_THREAD_NAMES;
+ if (input == switches::kMemlogStackModePseudo)
return profiling::mojom::StackMode::PSEUDO;
- if (stack_mode == switches::kMemlogStackModeMixed)
+ if (input == switches::kMemlogStackModeMixed)
return profiling::mojom::StackMode::MIXED;
- return profiling::mojom::StackMode::NATIVE;
+ DLOG(ERROR) << "Unsupported value: \"" << input << "\" passed to --"
+ << switches::kMemlogStackMode;
+ return profiling::mojom::StackMode::NATIVE_WITHOUT_THREAD_NAMES;
}
// static
ProfilingProcessHost* ProfilingProcessHost::Start(
content::ServiceManagerConnection* connection,
Mode mode,
- profiling::mojom::StackMode stack_mode) {
+ mojom::StackMode stack_mode) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
CHECK(!has_started_);
has_started_ = true;
diff --git a/chrome/browser/profiling_host/profiling_process_host.h b/chrome/browser/profiling_host/profiling_process_host.h
index d33d801..08e0fc6 100644
--- a/chrome/browser/profiling_host/profiling_process_host.h
+++ b/chrome/browser/profiling_host/profiling_process_host.h
@@ -100,14 +100,15 @@
// Returns the mode specified by the command line or via about://flags.
static Mode GetModeForStartup();
static Mode ConvertStringToMode(const std::string& input);
- static profiling::mojom::StackMode GetStackModeForStartup();
+ static mojom::StackMode GetStackModeForStartup();
+ static mojom::StackMode ConvertStringToStackMode(const std::string& input);
bool ShouldProfileNonRendererProcessType(int process_type);
// Launches the profiling process and returns a pointer to it.
static ProfilingProcessHost* Start(
content::ServiceManagerConnection* connection,
Mode mode,
- profiling::mojom::StackMode stack_mode);
+ mojom::StackMode stack_mode);
// Returns true if Start() has been called.
static bool has_started() { return has_started_; }
diff --git a/chrome/browser/profiling_host/profiling_test_driver.cc b/chrome/browser/profiling_host/profiling_test_driver.cc
index 730141c..d0954fad 100644
--- a/chrome/browser/profiling_host/profiling_test_driver.cc
+++ b/chrome/browser/profiling_host/profiling_test_driver.cc
@@ -12,6 +12,7 @@
#include "base/process/process_handle.h"
#include "base/run_loop.h"
#include "base/task_scheduler/post_task.h"
+#include "base/threading/platform_thread.h"
#include "base/trace_event/heap_profiler_event_filter.h"
#include "base/trace_event/trace_config_memory_test_util.h"
#include "base/values.h"
@@ -30,6 +31,7 @@
const char kMallocEvent[] = "kMallocEvent";
const char kPAEvent[] = "kPAEvent";
const char kVariadicEvent[] = "kVariadicEvent";
+const char kThreadName[] = "kThreadName";
// Make some specific allocations in Browser to do a deeper test of the
// allocation tracking.
@@ -124,9 +126,11 @@
return nullptr;
}
+constexpr uint64_t kNullParent = std::numeric_limits<int>::max();
struct Node {
- uint64_t name_id;
+ int name_id;
std::string name;
+ int parent_id = kNullParent;
};
using NodeMap = std::unordered_map<uint64_t, Node>;
@@ -143,6 +147,47 @@
Node node;
node.name_id = name_sid->GetInt();
+
+ const base::Value* parent_id = node_value.FindKey("parent");
+ if (parent_id) {
+ node.parent_id = parent_id->GetInt();
+ }
+
+ (*output)[id->GetInt()] = node;
+ }
+
+ base::Value* strings = heaps_v2->FindPath({"maps", "strings"});
+ for (const base::Value& string_value : strings->GetList()) {
+ const base::Value* id = string_value.FindKey("id");
+ const base::Value* string = string_value.FindKey("string");
+ if (!id || !string) {
+ LOG(ERROR) << "String struct missing id or string field";
+ return false;
+ }
+ for (auto& pair : *output) {
+ if (pair.second.name_id == id->GetInt()) {
+ pair.second.name = string->GetString();
+ break;
+ }
+ }
+ }
+
+ return true;
+}
+
+// Parses maps.types and maps.strings. Returns |true| on success.
+bool ParseTypes(base::Value* heaps_v2, NodeMap* output) {
+ base::Value* types = heaps_v2->FindPath({"maps", "types"});
+ for (const base::Value& type_value : types->GetList()) {
+ const base::Value* id = type_value.FindKey("id");
+ const base::Value* name_sid = type_value.FindKey("name_sid");
+ if (!id || !name_sid) {
+ LOG(ERROR) << "Node missing id or name_sid field";
+ return false;
+ }
+
+ Node node;
+ node.name_id = name_sid->GetInt();
(*output)[id->GetInt()] = node;
}
@@ -155,7 +200,7 @@
return false;
}
for (auto& pair : *output) {
- if (pair.second.name_id == static_cast<uint64_t>(id->GetInt())) {
+ if (pair.second.name_id == id->GetInt()) {
pair.second.name = string->GetString();
break;
}
@@ -171,7 +216,8 @@
int expected_alloc_count,
const char* allocator_name,
const char* type_name,
- const std::string& frame_name) {
+ const std::string& frame_name,
+ const std::string& thread_name) {
base::Value* sizes =
heaps_v2->FindPath({"allocators", allocator_name, "sizes"});
if (!sizes) {
@@ -268,25 +314,21 @@
// Find the type, if an expectation was passed in.
if (type_name) {
- bool found = false;
- int type = types_list[browser_alloc_index].GetInt();
- base::Value* strings = heaps_v2->FindPath({"maps", "strings"});
- for (const base::Value& dict : strings->GetList()) {
- // Each dict has the format {"id":1,"string":"kPartitionAllocTypeName"}
- int id = dict.FindKey("id")->GetInt();
- if (id == type) {
- found = true;
- std::string name = dict.FindKey("string")->GetString();
- if (name != type_name) {
- LOG(ERROR) << "actual name: " << name
- << " expected name: " << type_name;
- return false;
- }
- break;
- }
+ NodeMap node_map;
+ if (!ParseTypes(heaps_v2, &node_map)) {
+ LOG(ERROR) << "Failed to parse type and string structs";
+ return false;
}
- if (!found) {
- LOG(ERROR) << "Failed to find type name string: " << type_name;
+
+ int type = types_list[browser_alloc_index].GetInt();
+ auto it = node_map.find(type);
+ if (it == node_map.end()) {
+ LOG(ERROR) << "Failed to look up type.";
+ return false;
+ }
+ if (it->second.name != type_name) {
+ LOG(ERROR) << "actual name: " << it->second.name
+ << " expected name: " << type_name;
return false;
}
}
@@ -303,7 +345,7 @@
auto it = node_map.find(node_id);
if (it == node_map.end()) {
- LOG(ERROR) << "Failed to find root for node with id: " << node_id;
+ LOG(ERROR) << "Failed to find frame for node with id: " << node_id;
return false;
}
@@ -314,6 +356,34 @@
}
}
+ // Check that the thread [top frame] has the right name.
+ if (!thread_name.empty()) {
+ NodeMap node_map;
+ if (!ParseNodes(heaps_v2, &node_map)) {
+ LOG(ERROR) << "Failed to parse node and string structs";
+ return false;
+ }
+
+ int node_id = nodes_list[browser_alloc_index].GetInt();
+ auto it = node_map.find(node_id);
+ while (true) {
+ if (it == node_map.end() || it->second.parent_id == kNullParent)
+ break;
+ it = node_map.find(it->second.parent_id);
+ }
+
+ if (it == node_map.end()) {
+ LOG(ERROR) << "Failed to find root for node with id: " << node_id;
+ return false;
+ }
+
+ if (it->second.name != thread_name) {
+ LOG(ERROR) << "Wrong name: " << it->second.name
+ << " for thread with expected name: " << thread_name;
+ return false;
+ }
+ }
+
return true;
}
@@ -457,6 +527,8 @@
void ProfilingTestDriver::MakeTestAllocations() {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+ base::PlatformThread::SetName(kThreadName);
+
leaks_.reserve(2 * kMallocAllocCount + 1 + kPartitionAllocSize);
{
@@ -561,10 +633,12 @@
should_validate_dumps = false;
#endif
+ std::string thread_name = ShouldIncludeNativeThreadNames() ? kThreadName : "";
+
if (should_validate_dumps) {
result = ValidateDump(heaps_v2, kMallocAllocSize * kMallocAllocCount,
kMallocAllocCount, "malloc", nullptr,
- HasPseudoFrames() ? kMallocEvent : "");
+ HasPseudoFrames() ? kMallocEvent : "", thread_name);
if (!result) {
LOG(ERROR) << "Failed to validate malloc fixed allocations";
return false;
@@ -572,7 +646,7 @@
result = ValidateDump(heaps_v2, total_variadic_allocations_,
kVariadicAllocCount, "malloc", nullptr,
- HasPseudoFrames() ? kVariadicEvent : "");
+ HasPseudoFrames() ? kVariadicEvent : "", thread_name);
if (!result) {
LOG(ERROR) << "Failed to validate malloc variadic allocations";
return false;
@@ -584,10 +658,10 @@
// only one place that uses partition alloc in the browser process [this
// test], the count is still valid. This should still be made more robust by
// fixing backtrace. https://crbug.com/786450.
- result =
- ValidateDump(heaps_v2, kPartitionAllocSize * kPartitionAllocCount,
- kPartitionAllocCount, "partition_alloc",
- kPartitionAllocTypeName, HasPseudoFrames() ? kPAEvent : "");
+ result = ValidateDump(heaps_v2, kPartitionAllocSize * kPartitionAllocCount,
+ kPartitionAllocCount, "partition_alloc",
+ kPartitionAllocTypeName,
+ HasPseudoFrames() ? kPAEvent : "", thread_name);
if (!result) {
LOG(ERROR) << "Failed to validate PA allocations";
return false;
@@ -657,8 +731,13 @@
options_.mode == ProfilingProcessHost::Mode::kAllRenderers;
}
+bool ProfilingTestDriver::ShouldIncludeNativeThreadNames() {
+ return options_.stack_mode == mojom::StackMode::NATIVE_WITH_THREAD_NAMES;
+}
+
bool ProfilingTestDriver::HasPseudoFrames() {
- return options_.stack_mode != profiling::mojom::StackMode::NATIVE;
+ return options_.stack_mode == mojom::StackMode::PSEUDO ||
+ options_.stack_mode == mojom::StackMode::MIXED;
}
void ProfilingTestDriver::WaitForProfilingToStartForAllRenderersUIThread() {
diff --git a/chrome/browser/profiling_host/profiling_test_driver.h b/chrome/browser/profiling_host/profiling_test_driver.h
index 7a082e2a..8f960b7 100644
--- a/chrome/browser/profiling_host/profiling_test_driver.h
+++ b/chrome/browser/profiling_host/profiling_test_driver.h
@@ -47,9 +47,7 @@
profiling::mojom::StackMode stack_mode;
// Whether the caller has already started profiling with the given mode.
- // TODO(erikchen): Implement and test the case where this member is false.
- // Starting profiling is an asynchronous operation, so this requires adding
- // some more plumbing. https://crbug.com/753218.
+ // When false, the test driver is responsible for starting profiling.
bool profiling_already_started;
};
@@ -93,6 +91,7 @@
bool ShouldProfileBrowser();
bool ShouldProfileRenderer();
+ bool ShouldIncludeNativeThreadNames();
bool HasPseudoFrames();
void WaitForProfilingToStartForAllRenderersUIThread();
diff --git a/chrome/browser/profiling_host/test_android_shim.cc b/chrome/browser/profiling_host/test_android_shim.cc
index f1375e9..73b044d8 100644
--- a/chrome/browser/profiling_host/test_android_shim.cc
+++ b/chrome/browser/profiling_host/test_android_shim.cc
@@ -31,13 +31,14 @@
const base::android::JavaParamRef<jobject>& obj,
const base::android::JavaParamRef<jstring>& mode,
jboolean dynamically_start_profiling,
- jboolean pseudo_stacks) {
+ const base::android::JavaParamRef<jstring>& stack_mode) {
profiling::ProfilingTestDriver driver;
profiling::ProfilingTestDriver::Options options;
options.mode = profiling::ProfilingProcessHost::ConvertStringToMode(
base::android::ConvertJavaStringToUTF8(mode));
- options.stack_mode = pseudo_stacks ? profiling::mojom::StackMode::PSEUDO
- : profiling::mojom::StackMode::NATIVE;
+ options.stack_mode =
+ profiling::ProfilingProcessHost::ConvertStringToStackMode(
+ base::android::ConvertJavaStringToUTF8(stack_mode));
options.profiling_already_started = !dynamically_start_profiling;
return driver.RunTest(options);
}
diff --git a/chrome/browser/profiling_host/test_android_shim.h b/chrome/browser/profiling_host/test_android_shim.h
index c6d8345..8f96e16c 100644
--- a/chrome/browser/profiling_host/test_android_shim.h
+++ b/chrome/browser/profiling_host/test_android_shim.h
@@ -17,11 +17,12 @@
TestAndroidShim(JNIEnv* env, jobject obj);
void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
- jboolean RunTestForMode(JNIEnv* env,
- const base::android::JavaParamRef<jobject>& obj,
- const base::android::JavaParamRef<jstring>& mode,
- jboolean dynamically_start_profiling,
- jboolean pseudo_stacks);
+ jboolean RunTestForMode(
+ JNIEnv* env,
+ const base::android::JavaParamRef<jobject>& obj,
+ const base::android::JavaParamRef<jstring>& mode,
+ jboolean dynamically_start_profiling,
+ const base::android::JavaParamRef<jstring>& stack_mode);
private:
~TestAndroidShim();
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 4644fde..efbd4ae 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -482,6 +482,7 @@
const char kMemlogStackMode[] = "memlog-stack-mode";
const char kMemlogStackModeMixed[] = "mixed";
const char kMemlogStackModeNative[] = "native";
+const char kMemlogStackModeNativeWithThreadNames[] = "native-with-thread-names";
const char kMemlogStackModePseudo[] = "pseudo";
// Allows setting a different destination ID for connection-monitoring GCM
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 5adf77db..feb239f 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -159,6 +159,7 @@
extern const char kMemlogStackMode[];
extern const char kMemlogStackModeMixed[];
extern const char kMemlogStackModeNative[];
+extern const char kMemlogStackModeNativeWithThreadNames[];
extern const char kMemlogStackModePseudo[];
extern const char kMonitoringDestinationID[];
extern const char kNetLogCaptureMode[];
diff --git a/chrome/common/profiling/memlog_allocator_shim.cc b/chrome/common/profiling/memlog_allocator_shim.cc
index 57e92ab2..fd91937 100644
--- a/chrome/common/profiling/memlog_allocator_shim.cc
+++ b/chrome/common/profiling/memlog_allocator_shim.cc
@@ -14,6 +14,7 @@
#include "base/lazy_instance.h"
#include "base/numerics/safe_conversions.h"
#include "base/synchronization/lock.h"
+#include "base/threading/thread_id_name_manager.h"
#include "base/threading/thread_local.h"
#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
#include "base/trace_event/heap_profiler_allocation_register.h"
@@ -26,6 +27,10 @@
#include <limits.h>
#endif
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+#include <sys/prctl.h>
+#endif
+
using base::trace_event::AllocationContext;
using base::trace_event::AllocationContextTracker;
using CaptureMode = base::trace_event::AllocationContextTracker::CaptureMode;
@@ -42,6 +47,9 @@
MemlogSenderPipe* g_sender_pipe = nullptr;
+// In NATIVE stack mode, whether to insert stack names into the backtraces.
+bool g_include_thread_names = false;
+
// Prime since this is used like a hash table. Numbers of this magnitude seemed
// to provide sufficient parallelism to avoid lock overhead in ad-hoc testing.
constexpr int kNumSendBuffers = 17;
@@ -62,12 +70,85 @@
base::LazyInstance<base::ThreadLocalBoolean>::Leaky g_prevent_reentrancy =
LAZY_INSTANCE_INITIALIZER;
-// If we are using pseudo stacks, we need to inform the profiling service of the
-// address to string mapping. To avoid a global lock, we keep a thread-local
-// unordered_set of every address that has been sent from the thread in
-// question.
-base::LazyInstance<base::ThreadLocalPointer<std::unordered_set<const void*>>>::
- Leaky g_sent_strings = LAZY_INSTANCE_INITIALIZER;
+// The allocator shim needs to retain some additional state for each thread.
+struct ShimState {
+ // The pointer must be valid for the lifetime of the process.
+ const char* thread_name = nullptr;
+
+ // If we are using pseudo stacks, we need to inform the profiling service of
+ // the address to string mapping. To avoid a global lock, we keep a
+ // thread-local unordered_set of every address that has been sent from the
+ // thread in question.
+ std::unordered_set<const void*> sent_strings;
+};
+
+// This function is added to the TLS slot to clean up the instance when the
+// thread exits.
+void DestructShimState(void* shim_state) {
+ delete static_cast<ShimState*>(shim_state);
+}
+
+base::ThreadLocalStorage::StaticSlot g_tls_shim_state = TLS_INITIALIZER;
+
+// We don't need to worry about re-entrancy because g_prevent_reentrancy
+// already guards against that.
+ShimState* GetShimState() {
+ ShimState* state = static_cast<ShimState*>(g_tls_shim_state.Get());
+
+ if (!state) {
+ state = new ShimState();
+ g_tls_shim_state.Set(state);
+ }
+
+ return state;
+}
+
+// Set the thread name, which is a pointer to a leaked string, to ensure
+// validity forever.
+void SetCurrentThreadName(const char* name) {
+ GetShimState()->thread_name = name;
+}
+
+// Cannot call ThreadIdNameManager::GetName because it holds a lock and causes
+// deadlock when lock is already held by ThreadIdNameManager before the current
+// allocation. Gets the thread name from kernel if available or returns a string
+// with id. This function intentionally leaks the allocated strings since they
+// are used to tag allocations even after the thread dies.
+const char* GetAndLeakThreadName() {
+ // prctl requires 16 bytes, snprintf requires 19, pthread_getname_np requires
+ // 64 on macOS, see PlatformThread::SetName in platform_thread_mac.mm.
+ constexpr size_t kBufferLen = 64;
+ char name[kBufferLen];
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+ // If the thread name is not set, try to get it from prctl. Thread name might
+ // not be set in cases where the thread started before heap profiling was
+ // enabled.
+ int err = prctl(PR_GET_NAME, name);
+ if (!err) {
+ return strdup(name);
+ }
+#elif defined(OS_MACOSX)
+ int err = pthread_getname_np(pthread_self(), name, kBufferLen);
+ if (err == 0 && name[0] != '\0') {
+ return strdup(name);
+ }
+#endif // defined(OS_LINUX) || defined(OS_ANDROID)
+
+ // Use tid if we don't have a thread name.
+ snprintf(name, sizeof(name), "Thread %lu",
+ static_cast<unsigned long>(base::PlatformThread::CurrentId()));
+ return strdup(name);
+}
+
+// Returns the thread name, looking it up if necessary.
+const char* GetOrSetThreadName() {
+ const char* thread_name = GetShimState()->thread_name;
+ if (UNLIKELY(!thread_name)) {
+ thread_name = GetAndLeakThreadName();
+ GetShimState()->thread_name = thread_name;
+ }
+ return thread_name;
+}
class SendBuffer {
public:
@@ -298,6 +379,34 @@
AddInstructionPointer(frames[i]);
}
+ void AddCString(const char* c_string) {
+ // Using a TLS cache of sent_strings avoids lock contention on malloc, which
+ // would kill performance.
+ std::unordered_set<const void*>* sent_strings =
+ &GetShimState()->sent_strings;
+
+ if (sent_strings->find(c_string) == sent_strings->end()) {
+ // No point in allowing arbitrarily long c-strings, which might cause pipe
+ // max length issues. Pick a reasonable length like 255.
+ static const size_t kMaxCStringLen = 255;
+
+ // length does not include the null terminator.
+ size_t length = strnlen(c_string, kMaxCStringLen);
+
+ char message[sizeof(StringMappingPacket) + kMaxCStringLen];
+ StringMappingPacket* string_mapping_packet =
+ new (&message) StringMappingPacket();
+ string_mapping_packet->address = reinterpret_cast<uint64_t>(c_string);
+ string_mapping_packet->string_len = length;
+ memcpy(message + sizeof(StringMappingPacket), c_string, length);
+ DoSend(address_, message, sizeof(StringMappingPacket) + length,
+ send_buffers_);
+ sent_strings->insert(c_string);
+ }
+
+ AddInstructionPointer(c_string);
+ }
+
size_t count() { return count_; }
private:
@@ -307,38 +416,7 @@
return;
}
- // Using a TLS cache of sent_strings avoids lock contention on malloc, which
- // would kill performance.
- std::unordered_set<const void*>* sent_strings =
- g_sent_strings.Pointer()->Get();
-
- if (sent_strings == nullptr) {
- sent_strings = new std::unordered_set<const void*>;
- g_sent_strings.Pointer()->Set(sent_strings);
- }
-
- if (sent_strings->find(frame.value) == sent_strings->end()) {
- // No point in allowing arbitrarily long c-strings, which might cause pipe
- // max length issues. Pick a reasonable length like 255.
- static const size_t kMaxCStringLen = 255;
- const char* null_terminated_cstring =
- static_cast<const char*>(frame.value);
- // length does not include the null terminator.
- size_t length = strnlen(null_terminated_cstring, kMaxCStringLen);
-
- char message[sizeof(StringMappingPacket) + kMaxCStringLen];
- StringMappingPacket* string_mapping_packet =
- new (&message) StringMappingPacket();
- string_mapping_packet->address = reinterpret_cast<uint64_t>(frame.value);
- string_mapping_packet->string_len = length;
- memcpy(message + sizeof(StringMappingPacket), null_terminated_cstring,
- length);
- DoSend(address_, message, sizeof(StringMappingPacket) + length,
- send_buffers_);
- sent_strings->insert(frame.value);
- }
-
- AddInstructionPointer(frame.value);
+ AddCString(static_cast<const char*>(frame.value));
}
void AddInstructionPointer(const void* value) {
@@ -366,6 +444,8 @@
void InitTLSSlot() {
ignore_result(g_prevent_reentrancy.Pointer()->Get());
+ if (!g_tls_shim_state.initialized())
+ g_tls_shim_state.Initialize(DestructShimState);
}
// In order for pseudo stacks to work, trace event filtering must be enabled.
@@ -395,6 +475,12 @@
// Must be done before hooking any functions that make stack traces.
base::debug::EnableInProcessStackDumping();
+ if (stack_mode == mojom::StackMode::NATIVE_WITH_THREAD_NAMES) {
+ g_include_thread_names = true;
+ base::ThreadIdNameManager::GetInstance()->InstallSetNameCallback(
+ base::BindRepeating(&SetCurrentThreadName));
+ }
+
switch (stack_mode) {
case mojom::StackMode::PSEUDO:
EnableTraceEventFiltering();
@@ -404,7 +490,8 @@
EnableTraceEventFiltering();
AllocationContextTracker::SetCaptureMode(CaptureMode::MIXED_STACK);
break;
- case mojom::StackMode::NATIVE:
+ case mojom::StackMode::NATIVE_WITH_THREAD_NAMES:
+ case mojom::StackMode::NATIVE_WITHOUT_THREAD_NAMES:
AllocationContextTracker::SetCaptureMode(CaptureMode::DISABLED);
break;
}
@@ -464,19 +551,24 @@
void SerializeFramesFromBacktrace(FrameSerializer* serializer) {
#if BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
- const void* frames[kMaxStackEntries];
+ const void* frames[kMaxStackEntries - 1];
size_t frame_count = base::debug::TraceStackFramePointers(
- frames, kMaxStackEntries,
+ frames, kMaxStackEntries - 1,
1); // exclude this function from the trace.
#else // BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
// Fall-back to capturing the stack with base::debug::StackTrace,
// which is likely slower, but more reliable.
- base::debug::StackTrace stack_trace(kMaxStackEntries);
+ base::debug::StackTrace stack_trace(kMaxStackEntries - 1);
size_t frame_count = 0u;
const void* const* frames = stack_trace.Addresses(&frame_count);
#endif // BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
serializer->AddAllInstructionPointers(frame_count, frames);
+
+ if (g_include_thread_names) {
+ const char* thread_name = GetOrSetThreadName();
+ serializer->AddCString(thread_name);
+ }
}
void AllocatorShimLogAlloc(AllocatorType type,
diff --git a/chrome/common/profiling/memlog_allocator_shim.h b/chrome/common/profiling/memlog_allocator_shim.h
index 9f83d5c..683aee47 100644
--- a/chrome/common/profiling/memlog_allocator_shim.h
+++ b/chrome/common/profiling/memlog_allocator_shim.h
@@ -17,7 +17,9 @@
void InitTLSSlot();
// Begin profiling all allocations in the process. Send the results to
-// |sender_pipe|.
+// |sender_pipe|. |stack_mode| describes the type of stack to record for each
+// allocation. |include_thread_names| describes whether to insert thread names
+// into NATIVE stacks.
void InitAllocatorShim(MemlogSenderPipe* sender_pipe,
mojom::StackMode stack_mode);
diff --git a/chrome/common/profiling/profiling_client.mojom b/chrome/common/profiling/profiling_client.mojom
index 6aac963..555a9b2 100644
--- a/chrome/common/profiling/profiling_client.mojom
+++ b/chrome/common/profiling/profiling_client.mojom
@@ -6,8 +6,12 @@
// The data that is recorded for an allocation stack.
enum StackMode {
+ // Instruction addresses from unwinding the stack. Includes the thread name as
+ // the first frame.
+ NATIVE_WITH_THREAD_NAMES,
+
// Instruction addresses from unwinding the stack.
- NATIVE,
+ NATIVE_WITHOUT_THREAD_NAMES,
// Uses trace events as identifiers.
PSEUDO,
diff --git a/chrome/profiling/json_exporter.cc b/chrome/profiling/json_exporter.cc
index c3a7077..905f4f75 100644
--- a/chrome/profiling/json_exporter.cc
+++ b/chrome/profiling/json_exporter.cc
@@ -168,7 +168,8 @@
// Inserts or retrieves the ID for a string in the string table.
size_t AddOrGetString(const std::string& str, StringTable* string_table) {
- auto result = string_table->emplace(str, string_table->size());
+ // The lowest ID should be 1. The chrome://tracing UI doesn't handle ID 0.
+ auto result = string_table->emplace(str, string_table->size() + 1);
// "result.first" is an iterator into the map.
return result.first->second;
}
@@ -202,8 +203,9 @@
size_t AddOrGetBacktraceNode(BacktraceNode node,
BacktraceTable* backtrace_table) {
+ // The lowest ID should be 1. The chrome://tracing UI doesn't handle ID 0.
auto result =
- backtrace_table->emplace(std::move(node), backtrace_table->size());
+ backtrace_table->emplace(std::move(node), backtrace_table->size() + 1);
// "result.first" is an iterator into the map.
return result.first->second;
}
