Skip to content

Commit

Permalink
src: remove ContextEmbedderIndex::kBindingDataStoreIndex
Browse files Browse the repository at this point in the history
We can now get the binding data through the reference to the
realm directly, so remove it from the context embedder data
slot.

PR-URL: nodejs#48836
Reviewed-By: Chengzhong Wu <[email protected]>
  • Loading branch information
joyeecheung authored and UlisesGascon committed Aug 14, 2023
1 parent c53872a commit 02baba0
Show file tree
Hide file tree
Showing 16 changed files with 26 additions and 48 deletions.
3 changes: 1 addition & 2 deletions src/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,7 @@ void InitializeHttpParser(Local<Object> target,
Local<Context> context,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
BindingData* const binding_data =
realm->AddBindingData<BindingData>(context, target);
BindingData* const binding_data = realm->AddBindingData<BindingData>(target);
if (binding_data == nullptr) return;

Local<FunctionTemplate> t = NewFunctionTemplate(realm->isolate(), Parser::New);
Expand Down
4 changes: 2 additions & 2 deletions src/encoding_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void BindingData::Deserialize(Local<Context> context,
// Recreate the buffer in the constructor.
InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
BindingData* binding =
realm->AddBindingData<BindingData>(context, holder, casted_info);
realm->AddBindingData<BindingData>(holder, casted_info);
CHECK_NOT_NULL(binding);
}

Expand Down Expand Up @@ -232,7 +232,7 @@ void BindingData::CreatePerContextProperties(Local<Object> target,
Local<Context> context,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
realm->AddBindingData<BindingData>(context, target);
realm->AddBindingData<BindingData>(target);
}

void BindingData::RegisterTimerExternalReferences(
Expand Down
6 changes: 0 additions & 6 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -572,10 +572,6 @@ void Environment::AssignToContext(Local<v8::Context> context,
context->SetAlignedPointerInEmbedderData(ContextEmbedderIndex::kEnvironment,
this);
context->SetAlignedPointerInEmbedderData(ContextEmbedderIndex::kRealm, realm);
// Used to retrieve bindings
context->SetAlignedPointerInEmbedderData(
ContextEmbedderIndex::kBindingDataStoreIndex,
realm != nullptr ? realm->binding_data_store() : nullptr);

// ContextifyContexts will update this to a pointer to the native object.
context->SetAlignedPointerInEmbedderData(
Expand All @@ -598,8 +594,6 @@ void Environment::UnassignFromContext(Local<v8::Context> context) {
nullptr);
context->SetAlignedPointerInEmbedderData(ContextEmbedderIndex::kRealm,
nullptr);
context->SetAlignedPointerInEmbedderData(
ContextEmbedderIndex::kBindingDataStoreIndex, nullptr);
context->SetAlignedPointerInEmbedderData(
ContextEmbedderIndex::kContextifyContext, nullptr);
}
Expand Down
5 changes: 2 additions & 3 deletions src/node_blob.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void Blob::CreatePerContextProperties(Local<Object> target,
Local<Context> context,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
realm->AddBindingData<BlobBindingData>(context, target);
realm->AddBindingData<BlobBindingData>(target);
}

Local<FunctionTemplate> Blob::GetConstructorTemplate(Environment* env) {
Expand Down Expand Up @@ -535,8 +535,7 @@ void BlobBindingData::Deserialize(Local<Context> context,
DCHECK_EQ(index, BaseObject::kEmbedderType);
HandleScope scope(context->GetIsolate());
Realm* realm = Realm::GetCurrent(context);
BlobBindingData* binding =
realm->AddBindingData<BlobBindingData>(context, holder);
BlobBindingData* binding = realm->AddBindingData<BlobBindingData>(holder);
CHECK_NOT_NULL(binding);
}

Expand Down
1 change: 0 additions & 1 deletion src/node_context_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ enum ContextEmbedderIndex {
kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX,
kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX,
kBindingDataStoreIndex = NODE_BINDING_DATA_STORE_INDEX,
kAllowCodeGenerationFromStrings =
NODE_CONTEXT_ALLOW_CODE_GENERATION_FROM_STRINGS_INDEX,
kContextifyContext = NODE_CONTEXT_CONTEXTIFY_CONTEXT_INDEX,
Expand Down
4 changes: 2 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3155,7 +3155,7 @@ void BindingData::Deserialize(Local<Context> context,
Realm* realm = Realm::GetCurrent(context);
InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
BindingData* binding =
realm->AddBindingData<BindingData>(context, holder, casted_info);
realm->AddBindingData<BindingData>(holder, casted_info);
CHECK_NOT_NULL(binding);
}

Expand Down Expand Up @@ -3308,7 +3308,7 @@ static void CreatePerContextProperties(Local<Object> target,
Local<Context> context,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
realm->AddBindingData<BindingData>(context, target);
realm->AddBindingData<BindingData>(target);
}

BindingData* FSReqBase::binding_data() {
Expand Down
2 changes: 1 addition & 1 deletion src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3180,7 +3180,7 @@ void Initialize(Local<Object> target,
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);

Http2State* const state = realm->AddBindingData<Http2State>(context, target);
Http2State* const state = realm->AddBindingData<Http2State>(target);
if (state == nullptr) return;

#define SET_STATE_TYPEDARRAY(name, field) \
Expand Down
3 changes: 1 addition & 2 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1203,8 +1203,7 @@ void InitializeHttpParser(Local<Object> target,
Realm* realm = Realm::GetCurrent(context);
Environment* env = realm->env();
Isolate* isolate = env->isolate();
BindingData* const binding_data =
realm->AddBindingData<BindingData>(context, target);
BindingData* const binding_data = realm->AddBindingData<BindingData>(target);
if (binding_data == nullptr) return;

Local<FunctionTemplate> t = NewFunctionTemplate(isolate, Parser::New);
Expand Down
4 changes: 2 additions & 2 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ void BindingData::Deserialize(Local<Context> context,
v8::HandleScope scope(context->GetIsolate());
Realm* realm = Realm::GetCurrent(context);
// Recreate the buffer in the constructor.
BindingData* binding = realm->AddBindingData<BindingData>(context, holder);
BindingData* binding = realm->AddBindingData<BindingData>(holder);
CHECK_NOT_NULL(binding);
}

Expand Down Expand Up @@ -607,7 +607,7 @@ static void CreatePerContextProperties(Local<Object> target,
Local<Context> context,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
realm->AddBindingData<BindingData>(context, target);
realm->AddBindingData<BindingData>(target);
}

void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
Expand Down
17 changes: 5 additions & 12 deletions src/node_realm-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ inline T* Realm::GetBindingData(
// static
template <typename T>
inline T* Realm::GetBindingData(v8::Local<v8::Context> context) {
BindingDataStore* map =
static_cast<BindingDataStore*>(context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kBindingDataStoreIndex));
Realm* realm = GetCurrent(context);
DCHECK_NOT_NULL(realm);
BindingDataStore* map = realm->binding_data_store();
DCHECK_NOT_NULL(map);
constexpr size_t binding_index = static_cast<size_t>(T::binding_type_int);
static_assert(binding_index < std::tuple_size_v<BindingDataStore>);
Expand All @@ -81,10 +81,7 @@ inline T* Realm::GetBindingData(v8::Local<v8::Context> context) {
}

template <typename T, typename... Args>
inline T* Realm::AddBindingData(v8::Local<v8::Context> context,
v8::Local<v8::Object> target,
Args&&... args) {
DCHECK_EQ(GetCurrent(context), this);
inline T* Realm::AddBindingData(v8::Local<v8::Object> target, Args&&... args) {
// This won't compile if T is not a BaseObject subclass.
static_assert(std::is_base_of_v<BaseObject, T>);
// The binding data must be weak so that it won't keep the realm reachable
Expand All @@ -93,15 +90,11 @@ inline T* Realm::AddBindingData(v8::Local<v8::Context> context,
// reachable throughout the lifetime of the realm.
BaseObjectWeakPtr<T> item =
MakeWeakBaseObject<T>(this, target, std::forward<Args>(args)...);
DCHECK_EQ(context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kBindingDataStoreIndex),
&binding_data_store_);
constexpr size_t binding_index = static_cast<size_t>(T::binding_type_int);
static_assert(binding_index < std::tuple_size_v<BindingDataStore>);
// Should not insert the binding twice.
// Each slot is expected to be assigned only once.
CHECK(!binding_data_store_[binding_index]);
binding_data_store_[binding_index] = item;
DCHECK_EQ(GetBindingData<T>(context), item.get());
return item.get();
}

Expand Down
4 changes: 1 addition & 3 deletions src/node_realm.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,7 @@ class Realm : public MemoryRetainer {
// this scope can access the created T* object using
// GetBindingData<T>(args) later.
template <typename T, typename... Args>
T* AddBindingData(v8::Local<v8::Context> context,
v8::Local<v8::Object> target,
Args&&... args);
T* AddBindingData(v8::Local<v8::Object> target, Args&&... args);
template <typename T, typename U>
static inline T* GetBindingData(const v8::PropertyCallbackInfo<U>& info);
template <typename T>
Expand Down
4 changes: 2 additions & 2 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1355,7 +1355,7 @@ void BindingData::Deserialize(Local<Context> context,
// Recreate the buffer in the constructor.
InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
BindingData* binding =
realm->AddBindingData<BindingData>(context, holder, casted_info);
realm->AddBindingData<BindingData>(holder, casted_info);
CHECK_NOT_NULL(binding);
}

Expand All @@ -1369,7 +1369,7 @@ void CreatePerContextProperties(Local<Object> target,
Local<Context> context,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
realm->AddBindingData<BindingData>(context, target);
realm->AddBindingData<BindingData>(target);
}

void CreatePerIsolateProperties(IsolateData* isolate_data,
Expand Down
4 changes: 2 additions & 2 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void BindingData::Deserialize(v8::Local<v8::Context> context,
DCHECK_EQ(index, BaseObject::kEmbedderType);
v8::HandleScope scope(context->GetIsolate());
Realm* realm = Realm::GetCurrent(context);
BindingData* binding = realm->AddBindingData<BindingData>(context, holder);
BindingData* binding = realm->AddBindingData<BindingData>(holder);
CHECK_NOT_NULL(binding);
}

Expand Down Expand Up @@ -374,7 +374,7 @@ void BindingData::CreatePerContextProperties(Local<Object> target,
Local<Context> context,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
realm->AddBindingData<BindingData>(context, target);
realm->AddBindingData<BindingData>(target);
}

void BindingData::RegisterExternalReferences(
Expand Down
5 changes: 2 additions & 3 deletions src/node_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ void BindingData::Deserialize(Local<Context> context,
// Recreate the buffer in the constructor.
InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
BindingData* binding =
realm->AddBindingData<BindingData>(context, holder, casted_info);
realm->AddBindingData<BindingData>(holder, casted_info);
CHECK_NOT_NULL(binding);
}

Expand Down Expand Up @@ -422,8 +422,7 @@ void Initialize(Local<Object> target,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
Environment* env = realm->env();
BindingData* const binding_data =
realm->AddBindingData<BindingData>(context, target);
BindingData* const binding_data = realm->AddBindingData<BindingData>(target);
if (binding_data == nullptr) return;

SetMethodNoSideEffect(
Expand Down
3 changes: 1 addition & 2 deletions src/quic/bindingdata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ void BindingData::DecreaseAllocatedSize(size_t size) {
void BindingData::Initialize(Environment* env, Local<Object> target) {
SetMethod(env->context(), target, "setCallbacks", SetCallbacks);
SetMethod(env->context(), target, "flushPacketFreelist", FlushPacketFreelist);
Realm::GetCurrent(env->context())
->AddBindingData<BindingData>(env->context(), target);
Realm::GetCurrent(env->context())->AddBindingData<BindingData>(target);
}

void BindingData::RegisterExternalReferences(
Expand Down
5 changes: 2 additions & 3 deletions src/timers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ void BindingData::Deserialize(Local<Context> context,
v8::HandleScope scope(context->GetIsolate());
Realm* realm = Realm::GetCurrent(context);
// Recreate the buffer in the constructor.
BindingData* binding = realm->AddBindingData<BindingData>(context, holder);
BindingData* binding = realm->AddBindingData<BindingData>(holder);
CHECK_NOT_NULL(binding);
}

Expand Down Expand Up @@ -151,8 +151,7 @@ void BindingData::CreatePerContextProperties(Local<Object> target,
void* priv) {
Realm* realm = Realm::GetCurrent(context);
Environment* env = realm->env();
BindingData* const binding_data =
realm->AddBindingData<BindingData>(context, target);
BindingData* const binding_data = realm->AddBindingData<BindingData>(target);
if (binding_data == nullptr) return;

// TODO(joyeecheung): move these into BindingData.
Expand Down

0 comments on commit 02baba0

Please sign in to comment.