From 33ca8a4017b75d3c7e81f0f88760fe1871b016bf Mon Sep 17 00:00:00 2001
From: Dominik Inführ <dinfuehr@chromium.org>
Date: Thu, 21 May 2026 15:56:25 +0200
Subject: [PATCH] [profiler] Drop weak edges from JSWeakSet/JSWeakMap

For JSWeakMap (and JSWeakSet) we are emitting ephemeron edges for each
ephemeron pair: 1) table -> value 2) key -> value. In addition, we
also emitted weak edges from the table both to each key and each value.

In this CL we drop those weak edges since they were anyways a bit confusing in the heap snapshot and just duplicate the ephemeron edges.

Bug: 515086856
Change-Id: I09b9023a5e081da8b26621b0d3515c3661b5d409
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7867594
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107511}
---

diff --git a/src/profiler/heap-snapshot-generator.cc b/src/profiler/heap-snapshot-generator.cc
index d910e4b..a4b363f 100644
--- a/src/profiler/heap-snapshot-generator.cc
+++ b/src/profiler/heap-snapshot-generator.cc
@@ -1768,15 +1768,23 @@
     int value_index = EphemeronHashTable::EntryToValueIndex(i);
     Tagged<Object> key = table->get(key_index);
     Tagged<Object> value = table->get(value_index);
-    SetWeakReference(entry, key_index, key,
-                     table->OffsetOfElementAt(key_index));
-    SetWeakReference(entry, value_index, value,
-                     table->OffsetOfElementAt(value_index));
+    // This stops auto-generating edges for the key and value from the table.
+    // All edges are create by CreateEphmeronEdges() below.
+    MarkVisitedField(table->OffsetOfElementAt(key_index));
+    MarkVisitedField(table->OffsetOfElementAt(value_index));
     HeapEntry* key_entry = GetEntry(key);
     HeapEntry* value_entry = GetEntry(value);
     HeapEntry* table_entry = GetEntry(table);
-    if (key_entry && value_entry && !IsUndefined(key)) {
-      generator_->CreateEphemeronEdges(table_entry, key_entry, value_entry);
+    if (key_entry && !IsUndefined(key)) {
+#ifdef V8_ENABLE_HEAP_SNAPSHOT_VERIFY
+      if (generator_->verifier() != nullptr) {
+        generator_->verifier()->MarkReferenceCheckedWithoutChecking(
+            table, Cast<HeapObject>(key));
+      }
+#endif
+      if (value_entry) {
+        generator_->CreateEphemeronEdges(table_entry, key_entry, value_entry);
+      }
     }
   }
 }
diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc
index 479dc52..ce63e3e 100644
--- a/test/cctest/test-heap-profiler.cc
+++ b/test/cctest/test-heap-profiler.cc
@@ -31,6 +31,7 @@
 
 #include <memory>
 #include <optional>
+#include <string>
 #include <vector>
 
 #include "include/v8-function.h"
@@ -222,6 +223,28 @@
   return nullptr;
 }
 
+const v8::HeapGraphEdge* FindEphemeronEdge(v8::Isolate* isolate,
+                                           const v8::HeapGraphNode* table,
+                                           const v8::HeapGraphNode* key) {
+  v8::String::Utf8Value key_name(isolate, key->GetName());
+  std::string prefix = "part of key (" + std::string(*key_name) + " @" +
+                       std::to_string(key->GetId()) + ") -> value (";
+  std::string suffix =
+      ") pair in WeakMap (table @" + std::to_string(table->GetId()) + ")";
+
+  for (int i = 0, count = table->GetChildrenCount(); i < count; ++i) {
+    const v8::HeapGraphEdge* prop = table->GetChild(i);
+    if (prop->GetType() != v8::HeapGraphEdge::kInternal) continue;
+    v8::String::Utf8Value name(isolate, prop->GetName());
+    std::string name_str(*name);
+    if (name_str.find(prefix) != std::string::npos &&
+        name_str.find(suffix) != std::string::npos) {
+      return prop;
+    }
+  }
+  return nullptr;
+}
+
 // The following functions are not Wasm-specific, but are only used in a
 // Wasm-specific test. As long as this is the case we only define them if Wasm
 // is enabled to avoid warnings about unused functions.
@@ -933,16 +956,19 @@
   const v8::HeapGraphNode* ws_table =
       GetProperty(env.isolate(), ws, v8::HeapGraphEdge::kInternal, "table");
   CHECK_EQ(v8::HeapGraphNode::kArray, ws_table->GetType());
-  CHECK_GT(ws_table->GetChildrenCount(), 0);
-  int weak_entries = 0;
+  CHECK_EQ(3, ws_table->GetChildrenCount());
+  // Verify that WeakSet table has NO edges to k and v.
   for (int i = 0, count = ws_table->GetChildrenCount(); i < count; ++i) {
     const v8::HeapGraphEdge* prop = ws_table->GetChild(i);
-    if (prop->GetType() != v8::HeapGraphEdge::kWeak) continue;
-    if (k->GetId() == prop->GetToNode()->GetId()) {
-      ++weak_entries;
-    }
+    CHECK_NE(k->GetId(), prop->GetToNode()->GetId());
   }
-  CHECK_EQ(1, weak_entries);
+  const v8::HeapGraphEdge* edge_k =
+      FindEphemeronEdge(env.isolate(), ws_table, k);
+  const v8::HeapGraphEdge* edge_v =
+      FindEphemeronEdge(env.isolate(), ws_table, v);
+  CHECK_NOT_NULL(edge_k);
+  CHECK_NOT_NULL(edge_v);
+  CHECK_EQ(edge_k->GetToNode()->GetId(), edge_v->GetToNode()->GetId());
   const v8::HeapGraphNode* ws_s =
       GetProperty(env.isolate(), ws, v8::HeapGraphEdge::kProperty, "str");
   CHECK(ws_s);
@@ -958,16 +984,32 @@
       GetProperty(env.isolate(), wm, v8::HeapGraphEdge::kInternal, "table");
   CHECK_EQ(v8::HeapGraphNode::kArray, wm_table->GetType());
   CHECK_GT(wm_table->GetChildrenCount(), 0);
-  weak_entries = 0;
+  int wm_ephemeron_edges_to_v = 0;
   for (int i = 0, count = wm_table->GetChildrenCount(); i < count; ++i) {
     const v8::HeapGraphEdge* prop = wm_table->GetChild(i);
-    if (prop->GetType() != v8::HeapGraphEdge::kWeak) continue;
-    const v8::SnapshotObjectId to_node_id = prop->GetToNode()->GetId();
-    if (to_node_id == k->GetId() || to_node_id == v->GetId()) {
-      ++weak_entries;
+    // Verify NO edge to k.
+    CHECK_NE(k->GetId(), prop->GetToNode()->GetId());
+    if (prop->GetToNode()->GetId() == v->GetId()) {
+      CHECK_EQ(v8::HeapGraphEdge::kInternal, prop->GetType());
+      v8::String::Utf8Value prop_name(env.isolate(), prop->GetName());
+      CHECK_NOT_NULL(strstr(*prop_name, "pair in WeakMap"));
+      ++wm_ephemeron_edges_to_v;
     }
   }
-  CHECK_EQ(2, weak_entries);  // Key and value are weak.
+  CHECK_EQ(1, wm_ephemeron_edges_to_v);
+
+  // Verify that k has an ephemeron edge to v.
+  int k_ephemeron_edges_to_v = 0;
+  for (int i = 0, count = k->GetChildrenCount(); i < count; ++i) {
+    const v8::HeapGraphEdge* prop = k->GetChild(i);
+    if (prop->GetToNode()->GetId() == v->GetId()) {
+      CHECK_EQ(v8::HeapGraphEdge::kInternal, prop->GetType());
+      v8::String::Utf8Value prop_name(env.isolate(), prop->GetName());
+      CHECK_NOT_NULL(strstr(*prop_name, "pair in WeakMap"));
+      ++k_ephemeron_edges_to_v;
+    }
+  }
+  CHECK_EQ(1, k_ephemeron_edges_to_v);
   const v8::HeapGraphNode* wm_s =
       GetProperty(env.isolate(), wm, v8::HeapGraphEdge::kProperty, "str");
   CHECK(wm_s);
diff --git a/test/unittests/profiler/heap-snapshot-unittest.cc b/test/unittests/profiler/heap-snapshot-unittest.cc
index 3c4494c..0d1e357 100644
--- a/test/unittests/profiler/heap-snapshot-unittest.cc
+++ b/test/unittests/profiler/heap-snapshot-unittest.cc
@@ -4,6 +4,8 @@
 
 #include <json/json.h>
 
+#include "src/objects/hash-table-inl.h"
+#include "src/objects/js-collection-inl.h"
 #include "src/profiler/heap-profiler.h"
 #include "src/profiler/heap-snapshot-generator.h"
 #include "test/unittests/profiler/heap-snapshot-utils.h"
@@ -484,4 +486,72 @@
   EXPECT_STRNE("%s / %s", entry->name());
 }
 
+TEST_F(HeapSnapshotTest, EphemeronHashTableEdges) {
+  HandleScope scope(i_isolate());
+
+  v8::Local<v8::Value> weak_map_val =
+      RunJS("const weak_map = new WeakMap(); weak_map");
+  v8::Local<v8::Value> key_val = RunJS("const key = {name: 'my_key'}; key");
+  v8::Local<v8::Value> val_val = RunJS("const val = {name: 'my_val'}; val");
+
+  Handle<JSWeakMap> weak_map =
+      Cast<JSWeakMap>(v8::Utils::OpenHandle(*weak_map_val));
+  Handle<JSObject> key = Cast<JSObject>(v8::Utils::OpenHandle(*key_val));
+  Handle<JSObject> val = Cast<JSObject>(v8::Utils::OpenHandle(*val_val));
+
+  RunJS("weak_map.set(key, val);");
+
+  HeapSnapshot* snapshot = TakeHeapSnapshot();
+
+  const HeapEntry* weak_map_entry =
+      GetEntryFor(i_isolate(), snapshot, *weak_map);
+  const HeapEntry* key_entry = GetEntryFor(i_isolate(), snapshot, *key);
+  const HeapEntry* val_entry = GetEntryFor(i_isolate(), snapshot, *val);
+
+  ASSERT_NE(nullptr, weak_map_entry);

diff --git a/test/unittests/profiler/heap-snapshot-unittest.cc b/test/unittests/profiler/heap-snapshot-unittest.cc
index 3c4494c..0d1e357 100644
--- a/test/unittests/profiler/heap-snapshot-unittest.cc
+++ b/test/unittests/profiler/heap-snapshot-unittest.cc
@@ -4,6 +4,8 @@
 
 #include <json/json.h>
 
+#include "src/objects/hash-table-inl.h"
+#include "src/objects/js-collection-inl.h"
 #include "src/profiler/heap-profiler.h"
 #include "src/profiler/heap-snapshot-generator.h"
 #include "test/unittests/profiler/heap-snapshot-utils.h"
@@ -484,4 +486,72 @@
   EXPECT_STRNE("%s / %s", entry->name());
 }
 
+TEST_F(HeapSnapshotTest, EphemeronHashTableEdges) {
+  HandleScope scope(i_isolate());
+
+  v8::Local<v8::Value> weak_map_val =
+      RunJS("const weak_map = new WeakMap(); weak_map");
+  v8::Local<v8::Value> key_val = RunJS("const key = {name: 'my_key'}; key");
+  v8::Local<v8::Value> val_val = RunJS("const val = {name: 'my_val'}; val");
+
+  Handle<JSWeakMap> weak_map =
+      Cast<JSWeakMap>(v8::Utils::OpenHandle(*weak_map_val));
+  Handle<JSObject> key = Cast<JSObject>(v8::Utils::OpenHandle(*key_val));
+  Handle<JSObject> val = Cast<JSObject>(v8::Utils::OpenHandle(*val_val));
+
+  RunJS("weak_map.set(key, val);");
+
+  HeapSnapshot* snapshot = TakeHeapSnapshot();
+
+  const HeapEntry* weak_map_entry =
+      GetEntryFor(i_isolate(), snapshot, *weak_map);
+  const HeapEntry* key_entry = GetEntryFor(i_isolate(), snapshot, *key);
+  const HeapEntry* val_entry = GetEntryFor(i_isolate(), snapshot, *val);
+
+  ASSERT_NE(nullptr, weak_map_entry);
+  ASSERT_NE(nullptr, key_entry);
+  ASSERT_NE(nullptr, val_entry);
+
+  // The WeakMap has 3 edges: map + proto + table.
+  EXPECT_EQ(3, weak_map_entry->children_count());
+  EXPECT_EQ(nullptr, FindFirstEdgeTo(*weak_map_entry, *key_entry));
+  EXPECT_EQ(nullptr, FindFirstEdgeTo(*weak_map_entry, *val_entry));
+
+  Tagged<EphemeronHashTable> table =
+      Cast<EphemeronHashTable>(weak_map->table());
+  const HeapEntry* table_entry = GetEntryFor(i_isolate(), snapshot, table);
+  ASSERT_NE(nullptr, table_entry);
+
+  // There should be exactly 2 edges: 1) the map and 2) the ephemeron edge from
+  // table -> value.
+  EXPECT_EQ(2, table_entry->children_count());
+
+  // Verify that there are NO edges from table to key.
+  const HeapGraphEdge* table_to_key_edge =
+      FindFirstEdgeTo(*table_entry, *key_entry);
+  EXPECT_EQ(nullptr, table_to_key_edge);
+
+  std::string expected_name =
+      " / part of key (" + std::string(key_entry->name()) + " @" +
+      std::to_string(key_entry->id()) + ") -> value (" +
+      std::string(val_entry->name()) + " @" + std::to_string(val_entry->id()) +
+      ") pair in WeakMap (table @" + std::to_string(table_entry->id()) + ")";
+
+  // Verify that the only edge from table to val is the ephemeron edge.
+  const HeapGraphEdge* table_to_val_edge =
+      FindFirstEdgeTo(*table_entry, *val_entry);
+  ASSERT_NE(nullptr, table_to_val_edge);
+  EXPECT_EQ(HeapGraphEdge::kInternal, table_to_val_edge->type());
+  EXPECT_TRUE(
+      std::string_view(table_to_val_edge->name()).ends_with(expected_name));
+
+  // Verify that the edge from key to val is the ephemeron edge.
+  const HeapGraphEdge* key_to_val_edge =
+      FindFirstEdgeTo(*key_entry, *val_entry);
+  ASSERT_NE(nullptr, key_to_val_edge);
+  EXPECT_EQ(HeapGraphEdge::kInternal, key_to_val_edge->type());
+  EXPECT_TRUE(
+      std::string_view(key_to_val_edge->name()).ends_with(expected_name));
+}
+
 }  // namespace v8::internal