From 7d83f374c24a83b3519f8b460c040d8e6ecf6d71 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 29 Aug 2024 01:13:45 +0200
Subject: [PATCH] src: support v8::Data in heap utils

Use a std::set<> for saving the JSGraphJSNode, since implementing
a proper hash function for v8::Data is complicated and this
path is only used by tests anyway, where the performance difference
between std::set and std::unordered_set doesn't matter.

PR-URL: https://github.com/nodejs/node/pull/52295
Refs: https://github.com/nodejs/node/issues/40786
Refs: https://docs.google.com/document/d/1ny2Qz_EsUnXGKJRGxoA-FXIE2xpLgaMAN6jD7eAkqFQ/edit
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
---
 src/heap_utils.cc | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/src/heap_utils.cc b/src/heap_utils.cc
index f55255be5fd428..7b93698c7fe125 100644
--- a/src/heap_utils.cc
+++ b/src/heap_utils.cc
@@ -21,6 +21,7 @@
 using v8::Array;
 using v8::Boolean;
 using v8::Context;
+using v8::Data;
 using v8::EmbedderGraph;
 using v8::EscapableHandleScope;
 using v8::FunctionCallbackInfo;
@@ -50,42 +51,35 @@ class JSGraphJSNode : public EmbedderGraph::Node {
   const char* Name() override { return "<JS Node>"; }
   size_t SizeInBytes() override { return 0; }
   bool IsEmbedderNode() override { return false; }
-  Local<Value> JSValue() { return PersistentToLocal::Strong(persistent_); }
+  Local<Data> V8Value() { return PersistentToLocal::Strong(persistent_); }
 
-  int IdentityHash() {
-    Local<Value> v = JSValue();
-    if (v->IsObject()) return v.As<Object>()->GetIdentityHash();
-    if (v->IsName()) return v.As<v8::Name>()->GetIdentityHash();
-    if (v->IsInt32()) return v.As<v8::Int32>()->Value();
-    return 0;
-  }
-
-  JSGraphJSNode(Isolate* isolate, Local<Value> val)
-      : persistent_(isolate, val) {
+  JSGraphJSNode(Isolate* isolate, Local<Data> val) : persistent_(isolate, val) {
     CHECK(!val.IsEmpty());
   }
 
-  struct Hash {
-    inline size_t operator()(JSGraphJSNode* n) const {
-      return static_cast<size_t>(n->IdentityHash());
-    }
-  };
-
   struct Equal {
     inline bool operator()(JSGraphJSNode* a, JSGraphJSNode* b) const {
-      return a->JSValue()->SameValue(b->JSValue());
+      Local<Data> data_a = a->V8Value();
+      Local<Data> data_b = a->V8Value();
+      if (data_a->IsValue()) {
+        if (!data_b->IsValue()) {
+          return false;
+        }
+        return data_a.As<Value>()->SameValue(data_b.As<Value>());
+      }
+      return data_a == data_b;
     }
   };
 
  private:
-  Global<Value> persistent_;
+  Global<Data> persistent_;
 };
 
 class JSGraph : public EmbedderGraph {
  public:
   explicit JSGraph(Isolate* isolate) : isolate_(isolate) {}
 
-  Node* V8Node(const Local<Value>& value) override {
+  Node* V8Node(const Local<v8::Data>& value) override {
     std::unique_ptr<JSGraphJSNode> n { new JSGraphJSNode(isolate_, value) };
     auto it = engine_nodes_.find(n.get());
     if (it != engine_nodes_.end())
@@ -94,6 +88,10 @@ class JSGraph : public EmbedderGraph {
     return AddNode(std::unique_ptr<Node>(n.release()));
   }
 
+  Node* V8Node(const Local<v8::Value>& value) override {
+    return V8Node(value.As<v8::Data>());
+  }
+
   Node* AddNode(std::unique_ptr<Node> node) override {
     Node* n = node.get();
     nodes_.emplace(std::move(node));
@@ -154,8 +152,9 @@ class JSGraph : public EmbedderGraph {
         if (nodes->Set(context, i++, obj).IsNothing())
           return MaybeLocal<Array>();
         if (!n->IsEmbedderNode()) {
-          value = static_cast<JSGraphJSNode*>(n.get())->JSValue();
-          if (obj->Set(context, value_string, value).IsNothing())
+          Local<Data> data = static_cast<JSGraphJSNode*>(n.get())->V8Value();
+          if (data->IsValue() &&
+              obj->Set(context, value_string, data.As<Value>()).IsNothing())
             return MaybeLocal<Array>();
         }
       }
@@ -207,8 +206,7 @@ class JSGraph : public EmbedderGraph {
  private:
   Isolate* isolate_;
   std::unordered_set<std::unique_ptr<Node>> nodes_;
-  std::unordered_set<JSGraphJSNode*, JSGraphJSNode::Hash, JSGraphJSNode::Equal>
-      engine_nodes_;
+  std::set<JSGraphJSNode*, JSGraphJSNode::Equal> engine_nodes_;
   std::unordered_map<Node*, std::set<std::pair<const char*, Node*>>> edges_;
 };