Commit 4ccf0a4c authored by Benedikt Meurer's avatar Benedikt Meurer Committed by V8 LUCI CQ

[profiler] Use description for Symbols in Heap snapshots.

Previously we'd report all property edges with symbol names as <symbol>,
which was not very useful, especially with private class fields now
seeing more adoption.

Fixed: chromium:1232467
Change-Id: I53cf0811c4b83d016b988b687c6decbddd3c2fdd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3055309
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75962}
parent 2dd0dbe9
...@@ -74,6 +74,23 @@ const char* StringsStorage::GetVFormatted(const char* format, va_list args) { ...@@ -74,6 +74,23 @@ const char* StringsStorage::GetVFormatted(const char* format, va_list args) {
return AddOrDisposeString(str.begin(), len); return AddOrDisposeString(str.begin(), len);
} }
const char* StringsStorage::GetSymbol(Symbol sym) {
if (!sym.description().IsString()) {
return "<symbol>";
}
String description = String::cast(sym.description());
int length = std::min(FLAG_heap_snapshot_string_limit, description.length());
auto data = description.ToCString(DISALLOW_NULLS, ROBUST_STRING_TRAVERSAL, 0,
length, &length);
if (sym.is_private_name()) {
return AddOrDisposeString(data.release(), length);
}
auto str_length = 8 + length + 1 + 1;
auto str_result = NewArray<char>(str_length);
snprintf(str_result, str_length, "<symbol %s>", data.get());
return AddOrDisposeString(str_result, str_length - 1);
}
const char* StringsStorage::GetName(Name name) { const char* StringsStorage::GetName(Name name) {
if (name.IsString()) { if (name.IsString()) {
String str = String::cast(name); String str = String::cast(name);
...@@ -83,7 +100,7 @@ const char* StringsStorage::GetName(Name name) { ...@@ -83,7 +100,7 @@ const char* StringsStorage::GetName(Name name) {
DISALLOW_NULLS, ROBUST_STRING_TRAVERSAL, 0, length, &actual_length); DISALLOW_NULLS, ROBUST_STRING_TRAVERSAL, 0, length, &actual_length);
return AddOrDisposeString(data.release(), actual_length); return AddOrDisposeString(data.release(), actual_length);
} else if (name.IsSymbol()) { } else if (name.IsSymbol()) {
return "<symbol>"; return GetSymbol(Symbol::cast(name));
} }
return ""; return "";
} }
...@@ -106,7 +123,7 @@ const char* StringsStorage::GetConsName(const char* prefix, Name name) { ...@@ -106,7 +123,7 @@ const char* StringsStorage::GetConsName(const char* prefix, Name name) {
return AddOrDisposeString(cons_result, cons_length - 1); return AddOrDisposeString(cons_result, cons_length - 1);
} else if (name.IsSymbol()) { } else if (name.IsSymbol()) {
return "<symbol>"; return GetSymbol(Symbol::cast(name));
} }
return ""; return "";
} }
......
...@@ -16,6 +16,7 @@ namespace v8 { ...@@ -16,6 +16,7 @@ namespace v8 {
namespace internal { namespace internal {
class Name; class Name;
class Symbol;
// Provides a storage of strings allocated in C++ heap, to hold them // Provides a storage of strings allocated in C++ heap, to hold them
// forever, even if they disappear from JS heap or external storage. // forever, even if they disappear from JS heap or external storage.
...@@ -57,6 +58,7 @@ class V8_EXPORT_PRIVATE StringsStorage { ...@@ -57,6 +58,7 @@ class V8_EXPORT_PRIVATE StringsStorage {
base::CustomMatcherHashMap::Entry* GetEntry(const char* str, int len); base::CustomMatcherHashMap::Entry* GetEntry(const char* str, int len);
PRINTF_FORMAT(2, 0) PRINTF_FORMAT(2, 0)
const char* GetVFormatted(const char* format, va_list args); const char* GetVFormatted(const char* format, va_list args);
const char* GetSymbol(Symbol sym);
base::CustomMatcherHashMap names_; base::CustomMatcherHashMap names_;
base::Mutex mutex_; base::Mutex mutex_;
......
...@@ -2358,7 +2358,7 @@ TEST(HiddenPropertiesFastCase) { ...@@ -2358,7 +2358,7 @@ TEST(HiddenPropertiesFastCase) {
GetProperty(isolate, global, v8::HeapGraphEdge::kProperty, "c"); GetProperty(isolate, global, v8::HeapGraphEdge::kProperty, "c");
CHECK(c); CHECK(c);
const v8::HeapGraphNode* hidden_props = const v8::HeapGraphNode* hidden_props =
GetProperty(isolate, global, v8::HeapGraphEdge::kProperty, "<symbol>"); GetProperty(isolate, c, v8::HeapGraphEdge::kProperty, "<symbol key>");
CHECK(!hidden_props); CHECK(!hidden_props);
v8::Local<v8::Value> cHandle = v8::Local<v8::Value> cHandle =
...@@ -2377,10 +2377,32 @@ TEST(HiddenPropertiesFastCase) { ...@@ -2377,10 +2377,32 @@ TEST(HiddenPropertiesFastCase) {
c = GetProperty(isolate, global, v8::HeapGraphEdge::kProperty, "c"); c = GetProperty(isolate, global, v8::HeapGraphEdge::kProperty, "c");
CHECK(c); CHECK(c);
hidden_props = hidden_props =
GetProperty(isolate, c, v8::HeapGraphEdge::kProperty, "<symbol>"); GetProperty(isolate, c, v8::HeapGraphEdge::kProperty, "<symbol key>");
CHECK(hidden_props); CHECK(hidden_props);
} }
TEST(SymbolsAndPrivateClassFields) {
v8::Isolate* isolate = CcTest::isolate();
LocalContext env;
v8::HandleScope scope(isolate);
v8::HeapProfiler* heap_profiler = isolate->GetHeapProfiler();
CompileRun(
"class C { #private = this; [Symbol('MySymbol')] = this; };\n"
"c = new C;\n");
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
CHECK(ValidateSnapshot(snapshot));
const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
const v8::HeapGraphNode* c =
GetProperty(isolate, global, v8::HeapGraphEdge::kProperty, "c");
CHECK(c);
const v8::HeapGraphNode* prop;
prop = GetProperty(isolate, c, v8::HeapGraphEdge::kProperty, "#private");
CHECK(prop);
prop = GetProperty(isolate, c, v8::HeapGraphEdge::kProperty,
"<symbol MySymbol>");
CHECK(prop);
}
TEST(AccessorInfo) { TEST(AccessorInfo) {
LocalContext env; LocalContext env;
......
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