Skip to content

Commit

Permalink
vm: store MicrotaskQueue in ContextifyContext directly
Browse files Browse the repository at this point in the history 
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: #48982
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
  • Loading branch information
@joyeecheung @UlisesGascon
joyeecheung authored and UlisesGascon committed Sep 10, 2023
1 parent b51946e commit 6ee74be
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 100 deletions.
5 changes: 1 addition & 4 deletions lib/vm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const {

const {
ContextifyScript,
MicrotaskQueue,
makeContext,
constants,
measureMemory: _measureMemory,
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/module_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
Local<Module> module = obj->module_.Get(isolate);

ContextifyContext* contextify_context = obj->contextify_context_;
std::shared_ptr<MicrotaskQueue> microtask_queue;
MicrotaskQueue* microtask_queue = nullptr;
if (contextify_context != nullptr)
microtask_queue = contextify_context->microtask_queue();

Expand Down
85 changes: 21 additions & 64 deletions src/node_contextify.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,15 @@ Local<Name> Uint32ToName(Local<Context> context, uint32_t index) {
} // anonymous namespace

BaseObjectPtr<ContextifyContext> ContextifyContext::New(
Environment* env,
Local<Object> sandbox_obj,
const ContextOptions& options) {
Environment* env, Local<Object> sandbox_obj, ContextOptions* options) {
HandleScope scope(env->isolate());
Local<ObjectTemplate> 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<Context> v8_context;
Expand All @@ -132,19 +130,16 @@ BaseObjectPtr<ContextifyContext> 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<Object> wrapper,
Local<Context> 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.
Expand Down Expand Up @@ -240,7 +235,7 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
Local<Context> v8_context,
Environment* env,
Local<Object> 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
Expand Down Expand Up @@ -268,14 +263,14 @@ BaseObjectPtr<ContextifyContext> 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;
}

Expand Down Expand Up @@ -374,16 +369,14 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
CHECK(args[4]->IsBoolean());
options.allow_code_gen_wasm = args[4].As<Boolean>();

if (args[5]->IsObject() &&
!env->microtask_queue_ctor_template().IsEmpty() &&
env->microtask_queue_ctor_template()->HasInstance(args[5])) {
options.microtask_queue_wrap.reset(
Unwrap<MicrotaskQueueWrap>(args[5].As<Object>()));
if (args[5]->IsBoolean() && args[5]->BooleanValue(env->isolate())) {
options.own_microtask_queue =
MicrotaskQueue::New(env->isolate(), MicrotasksPolicy::kExplicit);
}

TryCatchScope try_catch(env);
BaseObjectPtr<ContextifyContext> context_ptr =
ContextifyContext::New(env, sandbox, options);
ContextifyContext::New(env, sandbox, &options);

if (try_catch.HasCaught()) {
if (!try_catch.HasTerminated())
Expand Down Expand Up @@ -987,7 +980,7 @@ void ContextifyScript::RunInContext(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsObject() || args[0]->IsNull());

Local<Context> context;
std::shared_ptr<MicrotaskQueue> microtask_queue;
v8::MicrotaskQueue* microtask_queue = nullptr;

if (args[0]->IsObject()) {
Local<Object> sandbox = args[0].As<Object>();
Expand Down Expand Up @@ -1036,7 +1029,7 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
const bool display_errors,
const bool break_on_sigint,
const bool break_on_first_line,
std::shared_ptr<MicrotaskQueue> mtask_queue,
MicrotaskQueue* mtask_queue,
const FunctionCallbackInfo<Value>& args) {
Context::Scope context_scope(context);

Expand Down Expand Up @@ -1068,7 +1061,7 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
bool received_signal = false;
auto run = [&]() {
MaybeLocal<Value> result = script->Run(context);
if (!result.IsEmpty() && mtask_queue)
if (!result.IsEmpty() && mtask_queue != nullptr)
mtask_queue->PerformCheckpoint(env->isolate());
return result;
};
Expand Down Expand Up @@ -1122,7 +1115,6 @@ bool ContextifyScript::EvalMachine(Local<Context> context,
return true;
}


ContextifyScript::ContextifyScript(Environment* env, Local<Object> object)
: BaseObject(env, object),
id_(env->get_next_script_id()) {
Expand Down Expand Up @@ -1376,46 +1368,12 @@ static void MeasureMemory(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(promise);
}

MicrotaskQueueWrap::MicrotaskQueueWrap(Environment* env, Local<Object> obj)
: BaseObject(env, obj),
microtask_queue_(
MicrotaskQueue::New(env->isolate(), MicrotasksPolicy::kExplicit)) {
MakeWeak();
}

const std::shared_ptr<MicrotaskQueue>&
MicrotaskQueueWrap::microtask_queue() const {
return microtask_queue_;
}

void MicrotaskQueueWrap::New(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
new MicrotaskQueueWrap(Environment::GetCurrent(args), args.This());
}

void MicrotaskQueueWrap::CreatePerIsolateProperties(
IsolateData* isolate_data, Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate();
HandleScope scope(isolate);
Local<FunctionTemplate> 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<ObjectTemplate> 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);
Expand Down Expand Up @@ -1470,7 +1428,6 @@ static void CreatePerContextProperties(Local<Object> target,
void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
ContextifyContext::RegisterExternalReferences(registry);
ContextifyScript::RegisterExternalReferences(registry);
MicrotaskQueueWrap::RegisterExternalReferences(registry);

registry->Register(StartSigintWatchdog);
registry->Register(StopSigintWatchdog);
Expand Down
39 changes: 8 additions & 31 deletions src/node_contextify.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,20 @@ class ExternalReferenceRegistry;

namespace contextify {

class MicrotaskQueueWrap : public BaseObject {
public:
MicrotaskQueueWrap(Environment* env, v8::Local<v8::Object> obj);

const std::shared_ptr<v8::MicrotaskQueue>& microtask_queue() const;

static void CreatePerIsolateProperties(IsolateData* isolate_data,
v8::Local<v8::ObjectTemplate> target);
static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
static void New(const v8::FunctionCallbackInfo<v8::Value>& 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<v8::MicrotaskQueue> microtask_queue_;
};

struct ContextOptions {
v8::Local<v8::String> name;
v8::Local<v8::String> origin;
v8::Local<v8::Boolean> allow_code_gen_strings;
v8::Local<v8::Boolean> allow_code_gen_wasm;
BaseObjectPtr<MicrotaskQueueWrap> microtask_queue_wrap;
std::unique_ptr<v8::MicrotaskQueue> own_microtask_queue;
};

class ContextifyContext : public BaseObject {
public:
ContextifyContext(Environment* env,
v8::Local<v8::Object> wrapper,
v8::Local<v8::Context> v8_context,
const ContextOptions& options);
ContextOptions* options);
~ContextifyContext();

void MemoryInfo(MemoryTracker* tracker) const override;
Expand Down Expand Up @@ -80,9 +58,8 @@ class ContextifyContext : public BaseObject {
.As<v8::Object>();
}

inline std::shared_ptr<v8::MicrotaskQueue> 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 <typename T>
Expand All @@ -94,12 +71,12 @@ class ContextifyContext : public BaseObject {
private:
static BaseObjectPtr<ContextifyContext> New(Environment* env,
v8::Local<v8::Object> sandbox_obj,
const ContextOptions& options);
ContextOptions* options);
// Initialize a context created from CreateV8Context()
static BaseObjectPtr<ContextifyContext> New(v8::Local<v8::Context> ctx,
Environment* env,
v8::Local<v8::Object> sandbox_obj,
const ContextOptions& options);
ContextOptions* options);

static bool IsStillInitializing(const ContextifyContext* ctx);
static void MakeContext(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down Expand Up @@ -146,7 +123,7 @@ class ContextifyContext : public BaseObject {
const v8::PropertyCallbackInfo<v8::Boolean>& args);

v8::Global<v8::Context> context_;
BaseObjectPtr<MicrotaskQueueWrap> microtask_queue_wrap_;
std::unique_ptr<v8::MicrotaskQueue> microtask_queue_;
};

class ContextifyScript : public BaseObject {
Expand All @@ -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<v8::MicrotaskQueue> microtask_queue,
v8::MicrotaskQueue* microtask_queue,
const v8::FunctionCallbackInfo<v8::Value>& args);

inline uint32_t id() { return id_; }
Expand Down

0 comments on commit 6ee74be

Please sign in to comment.