vm: migrate ContextifyScript to cppgc

Author: joyeecheung

benchmark/vm/compile-script-in-isolate-cache.js

@@ -5,17 +5,17 @@
 const common = require('../common.js');
 const fs = require('fs');
 const vm = require('vm');
-const fixtures = require('../../test/common/fixtures.js');
-const scriptPath = fixtures.path('snapshot', 'typescript.js');
+const path = require('path');
 
 const bench = common.createBenchmark(main, {
   type: ['with-dynamic-import-callback', 'without-dynamic-import-callback'],
-  n: [100],
+  filename: ['test/fixtures/snapshot/typescript.js', 'test/fixtures/syntax/good_syntax.js'],
+  n: [1000],
 });
 
-const scriptSource = fs.readFileSync(scriptPath, 'utf8');
-
-function main({ n, type }) {
+function main({ n, type, filename }) {
+  const scriptPath = path.resolve(__dirname, '..', '..', filename);
+  const scriptSource = fs.readFileSync(scriptPath, 'utf8');
   let script;
   bench.start();
   const options = {};

src/heap_utils.cc

@@ -18,6 +18,7 @@
 #endif
 
 using v8::Array;
+using v8::Data;
 using v8::Boolean;
 using v8::Context;
 using v8::EmbedderGraph;
@@ -49,17 +50,20 @@ 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();
+    Local<Data> d = V8Value();
+    // TODO(joyeecheung): return something better?
+    if (!d->IsValue()) return reinterpret_cast<std::uintptr_t>(this);
+    Local<Value> v = d.As<Value>();
     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)
+  JSGraphJSNode(Isolate* isolate, Local<Data> val)
       : persistent_(isolate, val) {
     CHECK(!val.IsEmpty());
   }
@@ -72,19 +76,27 @@ class JSGraphJSNode : public EmbedderGraph::Node {
 
   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())
@@ -153,8 +165,8 @@ 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>();
         }
       }

src/node_contextify.cc

@@ -21,6 +21,7 @@
 
 #include "node_contextify.h"
 
+#include "cppgc/allocation.h"
 #include "base_object-inl.h"
 #include "memory_tracker-inl.h"
 #include "module_wrap.h"
@@ -826,8 +827,9 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
     id_symbol = args[7].As<Symbol>();
   }
 
-  ContextifyScript* contextify_script =
-      new ContextifyScript(env, args.This());
+
+  ContextifyScript* contextify_script = cppgc::MakeGarbageCollected<ContextifyScript>(
+      env->isolate()->GetCppHeap()->GetAllocationHandle(), env, args.This());
 
   if (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
           TRACING_CATEGORY_NODE2(vm, script)) != 0) {
@@ -887,8 +889,6 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
   }
 
   contextify_script->script_.Reset(isolate, v8_script);
-  contextify_script->script_.SetWeak();
-  contextify_script->object()->SetInternalField(kUnboundScriptSlot, v8_script);
 
   std::unique_ptr<ScriptCompiler::CachedData> new_cached_data;
   if (produce_cached_data) {
@@ -990,10 +990,9 @@ bool ContextifyScript::InstanceOf(Environment* env,
 void ContextifyScript::CreateCachedData(
     const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
-  ContextifyScript* wrapped_script;
-  ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder());
-  Local<UnboundScript> unbound_script =
-      PersistentToLocal::Default(env->isolate(), wrapped_script->script_);
+  ContextifyScript* wrapped_script = CppgcMixin::Unwrap<ContextifyScript>(args.Holder());
+  CHECK_NOT_NULL(wrapped_script);
+  Local<UnboundScript> unbound_script = wrapped_script->script_.Get(env->isolate());
   std::unique_ptr<ScriptCompiler::CachedData> cached_data(
       ScriptCompiler::CreateCodeCache(unbound_script));
   if (!cached_data) {
@@ -1010,8 +1009,8 @@ void ContextifyScript::CreateCachedData(
 void ContextifyScript::RunInContext(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  ContextifyScript* wrapped_script;
-  ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder());
+  ContextifyScript* wrapped_script = CppgcMixin::Unwrap<ContextifyScript>(args.Holder());
+  CHECK_NOT_NULL(wrapped_script);
 
   CHECK_EQ(args.Length(), 5);
   CHECK(args[0]->IsObject() || args[0]->IsNull());
@@ -1081,10 +1080,9 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
 
   TryCatchScope try_catch(env);
   Isolate::SafeForTerminationScope safe_for_termination(env->isolate());
-  ContextifyScript* wrapped_script;
-  ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder(), false);
-  Local<UnboundScript> unbound_script =
-      PersistentToLocal::Default(env->isolate(), wrapped_script->script_);
+  ContextifyScript* wrapped_script = CppgcMixin::Unwrap<ContextifyScript>(args.Holder());
+  CHECK_NOT_NULL(wrapped_script);
+  Local<UnboundScript> unbound_script = wrapped_script->script_.Get(env->isolate());
   Local<Script> script = unbound_script->BindToCurrentContext();
 
 #if HAVE_INSPECTOR
@@ -1152,9 +1150,13 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
   return true;
 }
 
-ContextifyScript::ContextifyScript(Environment* env, Local<Object> object)
-    : BaseObject(env, object) {
-  MakeWeak();
+void ContextifyScript::Trace(cppgc::Visitor* visitor) const {
+  CppgcMixin::Trace(visitor);
+  visitor->Trace(script_);
+}
+
+ContextifyScript::ContextifyScript(Environment* env, Local<Object> object) {
+  InitializeCppgc(this, env, object);
 }
 
 ContextifyScript::~ContextifyScript() {}

src/node_contextify.h

@@ -6,6 +6,7 @@
 #include "base_object-inl.h"
 #include "node_context_data.h"
 #include "node_errors.h"
+#include "cppgc_helpers.h"
 
 namespace node {
 class ExternalReferenceRegistry;
@@ -152,16 +153,13 @@ class ContextifyContext : public BaseObject {
   std::unique_ptr<v8::MicrotaskQueue> microtask_queue_;
 };
 
-class ContextifyScript : public BaseObject {
+class ContextifyScript final
+    : public cppgc::GarbageCollected<ContextifyScript>,
+      public cppgc::NameProvider,
+      public CppgcMixin {
  public:
-  enum InternalFields {
-    kUnboundScriptSlot = BaseObject::kInternalFieldCount,
-    kInternalFieldCount
-  };
-
-  SET_NO_MEMORY_INFO()
-  SET_MEMORY_INFO_NAME(ContextifyScript)
-  SET_SELF_SIZE(ContextifyScript)
+  SET_CPPGC_NAME(ContextifyScript)
+  void Trace(cppgc::Visitor* visitor) const final;
 
   ContextifyScript(Environment* env, v8::Local<v8::Object> object);
   ~ContextifyScript() override;
@@ -183,7 +181,7 @@ class ContextifyScript : public BaseObject {
                           const v8::FunctionCallbackInfo<v8::Value>& args);
 
  private:
-  v8::Global<v8::UnboundScript> script_;
+  v8::TracedReference<v8::UnboundScript> script_;
 };
 
 v8::Maybe<bool> StoreCodeCacheResult(