From b4a2be457c17f32493801cbbec64942c0e70a068 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 16 Aug 2023 22:18:46 +0200 Subject: [PATCH] vm: store MicrotaskQueue in ContextifyContext directly Previously the ContextifyContext holds a MicrotaskQueueWrap which in turns holds a MicrotaskQueue in a shared pointer. The indirection is actually unnecessary, we can directly hold the MicrotaskQueue via a unique pointer in ContextifyContext, the lifetime would still remain the same but the graph would be simpler, and this removes the additional JS -> C++ to create the wrapper object. PR-URL: https://github.com/nodejs/node/pull/48982 Reviewed-By: Stephen Belanger --- lib/vm.js | 5 +-- src/module_wrap.cc | 2 +- src/node_contextify.cc | 85 +++++++++++------------------------------- src/node_contextify.h | 39 ++++--------------- 4 files changed, 31 insertions(+), 100 deletions(-) diff --git a/lib/vm.js b/lib/vm.js index b48e79c282541b..515c7afb4aedb9 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -30,7 +30,6 @@ const { const { ContextifyScript, - MicrotaskQueue, makeContext, constants, measureMemory: _measureMemory, @@ -238,9 +237,7 @@ function createContext(contextObject = {}, options = kEmptyObject) { validateOneOf(microtaskMode, 'options.microtaskMode', ['afterEvaluate', undefined]); - const microtaskQueue = microtaskMode === 'afterEvaluate' ? - new MicrotaskQueue() : - null; + const microtaskQueue = (microtaskMode === 'afterEvaluate'); makeContext(contextObject, name, origin, strings, wasm, microtaskQueue); return contextObject; diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 2dca349bd97089..0127a09167f851 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -361,7 +361,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { Local module = obj->module_.Get(isolate); ContextifyContext* contextify_context = obj->contextify_context_; - std::shared_ptr microtask_queue; + MicrotaskQueue* microtask_queue = nullptr; if (contextify_context != nullptr) microtask_queue = contextify_context->microtask_queue(); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index ee68ed12795740..a557f5bd9f3b35 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -110,17 +110,15 @@ Local Uint32ToName(Local context, uint32_t index) { } // anonymous namespace BaseObjectPtr ContextifyContext::New( - Environment* env, - Local sandbox_obj, - const ContextOptions& options) { + Environment* env, Local sandbox_obj, ContextOptions* options) { HandleScope scope(env->isolate()); Local object_template = env->contextify_global_template(); DCHECK(!object_template.IsEmpty()); const SnapshotData* snapshot_data = env->isolate_data()->snapshot_data(); MicrotaskQueue* queue = - options.microtask_queue_wrap - ? options.microtask_queue_wrap->microtask_queue().get() + options->own_microtask_queue + ? options->own_microtask_queue.get() : env->isolate()->GetCurrentContext()->GetMicrotaskQueue(); Local v8_context; @@ -132,19 +130,16 @@ BaseObjectPtr ContextifyContext::New( return New(v8_context, env, sandbox_obj, options); } -void ContextifyContext::MemoryInfo(MemoryTracker* tracker) const { - if (microtask_queue_wrap_) { - tracker->TrackField("microtask_queue_wrap", - microtask_queue_wrap_->object()); - } -} +void ContextifyContext::MemoryInfo(MemoryTracker* tracker) const {} ContextifyContext::ContextifyContext(Environment* env, Local wrapper, Local v8_context, - const ContextOptions& options) + ContextOptions* options) : BaseObject(env, wrapper), - microtask_queue_wrap_(options.microtask_queue_wrap) { + microtask_queue_(options->own_microtask_queue + ? options->own_microtask_queue.release() + : nullptr) { context_.Reset(env->isolate(), v8_context); // This should only be done after the initial initializations of the context // global object is finished. @@ -240,7 +235,7 @@ BaseObjectPtr ContextifyContext::New( Local v8_context, Environment* env, Local sandbox_obj, - const ContextOptions& options) { + ContextOptions* options) { HandleScope scope(env->isolate()); // This only initializes part of the context. The primordials are // only initialized when needed because even deserializing them slows @@ -268,14 +263,14 @@ BaseObjectPtr ContextifyContext::New( v8_context->AllowCodeGenerationFromStrings(false); v8_context->SetEmbedderData( ContextEmbedderIndex::kAllowCodeGenerationFromStrings, - options.allow_code_gen_strings); + options->allow_code_gen_strings); v8_context->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration, - options.allow_code_gen_wasm); + options->allow_code_gen_wasm); - Utf8Value name_val(env->isolate(), options.name); + Utf8Value name_val(env->isolate(), options->name); ContextInfo info(*name_val); - if (!options.origin.IsEmpty()) { - Utf8Value origin_val(env->isolate(), options.origin); + if (!options->origin.IsEmpty()) { + Utf8Value origin_val(env->isolate(), options->origin); info.origin = *origin_val; } @@ -374,16 +369,14 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo& args) { CHECK(args[4]->IsBoolean()); options.allow_code_gen_wasm = args[4].As(); - if (args[5]->IsObject() && - !env->microtask_queue_ctor_template().IsEmpty() && - env->microtask_queue_ctor_template()->HasInstance(args[5])) { - options.microtask_queue_wrap.reset( - Unwrap(args[5].As())); + if (args[5]->IsBoolean() && args[5]->BooleanValue(env->isolate())) { + options.own_microtask_queue = + MicrotaskQueue::New(env->isolate(), MicrotasksPolicy::kExplicit); } TryCatchScope try_catch(env); BaseObjectPtr context_ptr = - ContextifyContext::New(env, sandbox, options); + ContextifyContext::New(env, sandbox, &options); if (try_catch.HasCaught()) { if (!try_catch.HasTerminated()) @@ -987,7 +980,7 @@ void ContextifyScript::RunInContext(const FunctionCallbackInfo& args) { CHECK(args[0]->IsObject() || args[0]->IsNull()); Local context; - std::shared_ptr microtask_queue; + v8::MicrotaskQueue* microtask_queue = nullptr; if (args[0]->IsObject()) { Local sandbox = args[0].As(); @@ -1036,7 +1029,7 @@ bool ContextifyScript::EvalMachine(Local context, const bool display_errors, const bool break_on_sigint, const bool break_on_first_line, - std::shared_ptr mtask_queue, + MicrotaskQueue* mtask_queue, const FunctionCallbackInfo& args) { Context::Scope context_scope(context); @@ -1068,7 +1061,7 @@ bool ContextifyScript::EvalMachine(Local context, bool received_signal = false; auto run = [&]() { MaybeLocal result = script->Run(context); - if (!result.IsEmpty() && mtask_queue) + if (!result.IsEmpty() && mtask_queue != nullptr) mtask_queue->PerformCheckpoint(env->isolate()); return result; }; @@ -1122,7 +1115,6 @@ bool ContextifyScript::EvalMachine(Local context, return true; } - ContextifyScript::ContextifyScript(Environment* env, Local object) : BaseObject(env, object), id_(env->get_next_script_id()) { @@ -1376,46 +1368,12 @@ static void MeasureMemory(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(promise); } -MicrotaskQueueWrap::MicrotaskQueueWrap(Environment* env, Local obj) - : BaseObject(env, obj), - microtask_queue_( - MicrotaskQueue::New(env->isolate(), MicrotasksPolicy::kExplicit)) { - MakeWeak(); -} - -const std::shared_ptr& -MicrotaskQueueWrap::microtask_queue() const { - return microtask_queue_; -} - -void MicrotaskQueueWrap::New(const FunctionCallbackInfo& args) { - CHECK(args.IsConstructCall()); - new MicrotaskQueueWrap(Environment::GetCurrent(args), args.This()); -} - -void MicrotaskQueueWrap::CreatePerIsolateProperties( - IsolateData* isolate_data, Local target) { - Isolate* isolate = isolate_data->isolate(); - HandleScope scope(isolate); - Local tmpl = NewFunctionTemplate(isolate, New); - tmpl->InstanceTemplate()->SetInternalFieldCount( - ContextifyScript::kInternalFieldCount); - isolate_data->set_microtask_queue_ctor_template(tmpl); - SetConstructorFunction(isolate, target, "MicrotaskQueue", tmpl); -} - -void MicrotaskQueueWrap::RegisterExternalReferences( - ExternalReferenceRegistry* registry) { - registry->Register(New); -} - void CreatePerIsolateProperties(IsolateData* isolate_data, Local target) { Isolate* isolate = isolate_data->isolate(); ContextifyContext::CreatePerIsolateProperties(isolate_data, target); ContextifyScript::CreatePerIsolateProperties(isolate_data, target); - MicrotaskQueueWrap::CreatePerIsolateProperties(isolate_data, target); SetMethod(isolate, target, "startSigintWatchdog", StartSigintWatchdog); SetMethod(isolate, target, "stopSigintWatchdog", StopSigintWatchdog); @@ -1470,7 +1428,6 @@ static void CreatePerContextProperties(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { ContextifyContext::RegisterExternalReferences(registry); ContextifyScript::RegisterExternalReferences(registry); - MicrotaskQueueWrap::RegisterExternalReferences(registry); registry->Register(StartSigintWatchdog); registry->Register(StopSigintWatchdog); diff --git a/src/node_contextify.h b/src/node_contextify.h index 9a0cbe07d6e660..2bcc15b5f55ad3 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -12,34 +12,12 @@ class ExternalReferenceRegistry; namespace contextify { -class MicrotaskQueueWrap : public BaseObject { - public: - MicrotaskQueueWrap(Environment* env, v8::Local obj); - - const std::shared_ptr& microtask_queue() const; - - static void CreatePerIsolateProperties(IsolateData* isolate_data, - v8::Local target); - static void RegisterExternalReferences(ExternalReferenceRegistry* registry); - static void New(const v8::FunctionCallbackInfo& args); - - // This could have methods for running the microtask queue, if we ever decide - // to make that fully customizable from userland. - - SET_NO_MEMORY_INFO() - SET_MEMORY_INFO_NAME(MicrotaskQueueWrap) - SET_SELF_SIZE(MicrotaskQueueWrap) - - private: - std::shared_ptr microtask_queue_; -}; - struct ContextOptions { v8::Local name; v8::Local origin; v8::Local allow_code_gen_strings; v8::Local allow_code_gen_wasm; - BaseObjectPtr microtask_queue_wrap; + std::unique_ptr own_microtask_queue; }; class ContextifyContext : public BaseObject { @@ -47,7 +25,7 @@ class ContextifyContext : public BaseObject { ContextifyContext(Environment* env, v8::Local wrapper, v8::Local v8_context, - const ContextOptions& options); + ContextOptions* options); ~ContextifyContext(); void MemoryInfo(MemoryTracker* tracker) const override; @@ -80,9 +58,8 @@ class ContextifyContext : public BaseObject { .As(); } - inline std::shared_ptr microtask_queue() const { - if (!microtask_queue_wrap_) return {}; - return microtask_queue_wrap_->microtask_queue(); + inline v8::MicrotaskQueue* microtask_queue() const { + return microtask_queue_.get(); } template @@ -94,12 +71,12 @@ class ContextifyContext : public BaseObject { private: static BaseObjectPtr New(Environment* env, v8::Local sandbox_obj, - const ContextOptions& options); + ContextOptions* options); // Initialize a context created from CreateV8Context() static BaseObjectPtr New(v8::Local ctx, Environment* env, v8::Local sandbox_obj, - const ContextOptions& options); + ContextOptions* options); static bool IsStillInitializing(const ContextifyContext* ctx); static void MakeContext(const v8::FunctionCallbackInfo& args); @@ -146,7 +123,7 @@ class ContextifyContext : public BaseObject { const v8::PropertyCallbackInfo& args); v8::Global context_; - BaseObjectPtr microtask_queue_wrap_; + std::unique_ptr microtask_queue_; }; class ContextifyScript : public BaseObject { @@ -171,7 +148,7 @@ class ContextifyScript : public BaseObject { const bool display_errors, const bool break_on_sigint, const bool break_on_first_line, - std::shared_ptr microtask_queue, + v8::MicrotaskQueue* microtask_queue, const v8::FunctionCallbackInfo& args); inline uint32_t id() { return id_; }