From 2a35383b3e4106818b870905763f2403c524150b Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 3 Aug 2023 16:50:23 +0200 Subject: [PATCH] src: use per-realm GetBindingData() wherever applicable This reduce the number of embedder slot accesses and also removes the assumption in a few binding methods that the current realm is the principal realm of the current environment (which is not true for shadow realms). PR-URL: https://github.com/nodejs/node/pull/49007 Refs: https://github.com/nodejs/node/pull/48836 Reviewed-By: Chengzhong Wu Reviewed-By: Matteo Collina Reviewed-By: Rafael Gonzaga Reviewed-By: Stephen Belanger Reviewed-By: Yagiz Nizipli --- src/dataqueue/queue.cc | 12 +++++----- src/encoding_binding.cc | 7 +++--- src/node_blob.cc | 52 ++++++++++++++++++++--------------------- src/node_file-inl.h | 7 +++--- src/node_file.cc | 8 +++---- src/node_http2.cc | 10 ++++---- src/node_realm.h | 2 ++ src/node_url.cc | 27 ++++++++++----------- src/quic/bindingdata.cc | 2 +- 9 files changed, 63 insertions(+), 64 deletions(-) diff --git a/src/dataqueue/queue.cc b/src/dataqueue/queue.cc index 8ae28f9d0a791b..994b82a8751f6e 100644 --- a/src/dataqueue/queue.cc +++ b/src/dataqueue/queue.cc @@ -876,12 +876,12 @@ class FdEntry final : public EntryImpl { } Realm* realm = entry->env()->principal_realm(); return std::make_shared( - BaseObjectPtr(fs::FileHandle::New( - realm->GetBindingData(realm->context()), - file, - Local(), - entry->start_, - entry->end_ - entry->start_)), + BaseObjectPtr( + fs::FileHandle::New(realm->GetBindingData(), + file, + Local(), + entry->start_, + entry->end_ - entry->start_)), entry); } diff --git a/src/encoding_binding.cc b/src/encoding_binding.cc index c67af5319c8ff5..97ddd59fb661c8 100644 --- a/src/encoding_binding.cc +++ b/src/encoding_binding.cc @@ -83,12 +83,13 @@ void BindingData::Deserialize(Local context, } void BindingData::EncodeInto(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); CHECK_GE(args.Length(), 2); CHECK(args[0]->IsString()); CHECK(args[1]->IsUint8Array()); - BindingData* binding_data = Realm::GetBindingData(args); + + Realm* realm = Realm::GetCurrent(args); + Isolate* isolate = realm->isolate(); + BindingData* binding_data = realm->GetBindingData(); Local source = args[0].As(); diff --git a/src/node_blob.cc b/src/node_blob.cc index 8dc81a6f15e867..9ea37853ce1d46 100644 --- a/src/node_blob.cc +++ b/src/node_blob.cc @@ -400,20 +400,22 @@ std::unique_ptr Blob::CloneForMessaging() const { } void Blob::StoreDataObject(const v8::FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - BlobBindingData* binding_data = Realm::GetBindingData(args); + Realm* realm = Realm::GetCurrent(args); CHECK(args[0]->IsString()); // ID key - CHECK(Blob::HasInstance(env, args[1])); // Blob + CHECK(Blob::HasInstance(realm->env(), args[1])); // Blob CHECK(args[2]->IsUint32()); // Length CHECK(args[3]->IsString()); // Type - Utf8Value key(env->isolate(), args[0]); + BlobBindingData* binding_data = realm->GetBindingData(); + Isolate* isolate = realm->isolate(); + + Utf8Value key(isolate, args[0]); Blob* blob; ASSIGN_OR_RETURN_UNWRAP(&blob, args[1]); size_t length = args[2].As()->Value(); - Utf8Value type(env->isolate(), args[3]); + Utf8Value type(isolate, args[3]); binding_data->store_data_object( std::string(*key, key.length()), @@ -427,9 +429,11 @@ void Blob::StoreDataObject(const v8::FunctionCallbackInfo& args) { void Blob::RevokeObjectURL(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); CHECK(args[0]->IsString()); - BlobBindingData* binding_data = Realm::GetBindingData(args); - Environment* env = Environment::GetCurrent(args); - Utf8Value input(env->isolate(), args[0].As()); + Realm* realm = Realm::GetCurrent(args); + BlobBindingData* binding_data = realm->GetBindingData(); + Isolate* isolate = realm->isolate(); + + Utf8Value input(isolate, args[0].As()); auto out = ada::parse(input.ToStringView()); if (!out) { @@ -449,36 +453,30 @@ void Blob::RevokeObjectURL(const FunctionCallbackInfo& args) { } void Blob::GetDataObject(const v8::FunctionCallbackInfo& args) { - BlobBindingData* binding_data = Realm::GetBindingData(args); - - Environment* env = Environment::GetCurrent(args); CHECK(args[0]->IsString()); + Realm* realm = Realm::GetCurrent(args); + BlobBindingData* binding_data = realm->GetBindingData(); + Isolate* isolate = realm->isolate(); - Utf8Value key(env->isolate(), args[0]); + Utf8Value key(isolate, args[0]); BlobBindingData::StoredDataObject stored = binding_data->get_data_object(std::string(*key, key.length())); if (stored.blob) { Local type; - if (!String::NewFromUtf8( - env->isolate(), - stored.type.c_str(), - v8::NewStringType::kNormal, - static_cast(stored.type.length())).ToLocal(&type)) { + if (!String::NewFromUtf8(isolate, + stored.type.c_str(), + v8::NewStringType::kNormal, + static_cast(stored.type.length())) + .ToLocal(&type)) { return; } - Local values[] = { - stored.blob->object(), - Uint32::NewFromUnsigned(env->isolate(), stored.length), - type - }; + Local values[] = {stored.blob->object(), + Uint32::NewFromUnsigned(isolate, stored.length), + type}; - args.GetReturnValue().Set( - Array::New( - env->isolate(), - values, - arraysize(values))); + args.GetReturnValue().Set(Array::New(isolate, values, arraysize(values))); } } diff --git a/src/node_file-inl.h b/src/node_file-inl.h index 2ba5906d614f1c..cdf21a4b3a6c22 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -277,9 +277,10 @@ FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo& args, return Unwrap(value.As()); } - BindingData* binding_data = Realm::GetBindingData(args); - Environment* env = binding_data->env(); - if (value->StrictEquals(env->fs_use_promises_symbol())) { + Realm* realm = Realm::GetCurrent(args); + BindingData* binding_data = realm->GetBindingData(); + + if (value->StrictEquals(realm->isolate_data()->fs_use_promises_symbol())) { if (use_bigint) { return FSReqPromise::New(binding_data, use_bigint); } else { diff --git a/src/node_file.cc b/src/node_file.cc index a94792e7e96b1f..285e532f0078e1 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2105,14 +2105,14 @@ static void Open(const FunctionCallbackInfo& args) { } static void OpenFileHandle(const FunctionCallbackInfo& args) { - BindingData* binding_data = Realm::GetBindingData(args); - Environment* env = binding_data->env(); - Isolate* isolate = env->isolate(); + Realm* realm = Realm::GetCurrent(args); + BindingData* binding_data = realm->GetBindingData(); + Environment* env = realm->env(); const int argc = args.Length(); CHECK_GE(argc, 3); - BufferValue path(isolate, args[0]); + BufferValue path(realm->isolate(), args[0]); CHECK_NOT_NULL(*path); CHECK(args[1]->IsInt32()); diff --git a/src/node_http2.cc b/src/node_http2.cc index 0a8f2271f25689..070b40ae0a6ad6 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2650,12 +2650,12 @@ void Http2Session::RefreshState(const FunctionCallbackInfo& args) { // Constructor for new Http2Session instances. void Http2Session::New(const FunctionCallbackInfo& args) { - Http2State* state = Realm::GetBindingData(args); - Environment* env = state->env(); + Realm* realm = Realm::GetCurrent(args); + Http2State* state = realm->GetBindingData(); + CHECK(args.IsConstructCall()); - SessionType type = - static_cast( - args[0]->Int32Value(env->context()).ToChecked()); + SessionType type = static_cast( + args[0]->Int32Value(realm->context()).ToChecked()); Http2Session* session = new Http2Session(state, args.This(), type); Debug(session, "session created"); } diff --git a/src/node_realm.h b/src/node_realm.h index a75cd610692183..51fbd502a10eb6 100644 --- a/src/node_realm.h +++ b/src/node_realm.h @@ -101,6 +101,8 @@ class Realm : public MemoryRetainer { const v8::FunctionCallbackInfo& info); template static inline T* GetBindingData(v8::Local context); + template + inline T* GetBindingData(); inline BindingDataStore* binding_data_store(); // The BaseObject count is a debugging helper that makes sure that there are diff --git a/src/node_url.cc b/src/node_url.cc index da8790c1d1843e..85147ccd1c0d59 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -229,17 +229,16 @@ void BindingData::Parse(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); // input // args[1] // base url - BindingData* binding_data = Realm::GetBindingData(args); - Environment* env = Environment::GetCurrent(args); - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); + Realm* realm = Realm::GetCurrent(args); + BindingData* binding_data = realm->GetBindingData(); + Isolate* isolate = realm->isolate(); - Utf8Value input(env->isolate(), args[0]); + Utf8Value input(isolate, args[0]); ada::result base; ada::url_aggregator* base_pointer = nullptr; if (args[1]->IsString()) { - base = ada::parse( - Utf8Value(env->isolate(), args[1]).ToString()); + base = + ada::parse(Utf8Value(isolate, args[1]).ToString()); if (!base) { return args.GetReturnValue().Set(false); } @@ -255,8 +254,7 @@ void BindingData::Parse(const FunctionCallbackInfo& args) { binding_data->UpdateComponents(out->get_components(), out->type); args.GetReturnValue().Set( - ToV8Value(env->context(), out->get_href(), env->isolate()) - .ToLocalChecked()); + ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked()); } void BindingData::Update(const FunctionCallbackInfo& args) { @@ -264,12 +262,12 @@ void BindingData::Update(const FunctionCallbackInfo& args) { CHECK(args[1]->IsNumber()); // action type CHECK(args[2]->IsString()); // new value - BindingData* binding_data = Realm::GetBindingData(args); - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); + Realm* realm = Realm::GetCurrent(args); + BindingData* binding_data = realm->GetBindingData(); + Isolate* isolate = realm->isolate(); enum url_update_action action = static_cast( - args[1]->Uint32Value(env->context()).FromJust()); + args[1]->Uint32Value(realm->context()).FromJust()); Utf8Value input(isolate, args[0].As()); Utf8Value new_value(isolate, args[2].As()); @@ -330,8 +328,7 @@ void BindingData::Update(const FunctionCallbackInfo& args) { binding_data->UpdateComponents(out->get_components(), out->type); args.GetReturnValue().Set( - ToV8Value(env->context(), out->get_href(), env->isolate()) - .ToLocalChecked()); + ToV8Value(realm->context(), out->get_href(), isolate).ToLocalChecked()); } void BindingData::UpdateComponents(const ada::url_components& components, diff --git a/src/quic/bindingdata.cc b/src/quic/bindingdata.cc index af3642c1c16f7e..c97d781ca54ad9 100644 --- a/src/quic/bindingdata.cc +++ b/src/quic/bindingdata.cc @@ -25,7 +25,7 @@ using v8::Value; namespace quic { BindingData& BindingData::Get(Environment* env) { - return *Realm::GetBindingData(env->context()); + return *(env->principal_realm()->GetBindingData()); } BindingData::operator ngtcp2_mem() {