FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: store native async execution resources as `v8::Local` · nodejs/node@250e197 · GitHub

/ node Public

Commit 250e197

Browse files
addaleax
authored and
targos
committed
src: store native async execution resources as v8::Local
This is possible because the async stack is always expected to match the native call stack, and saves performance overhead that comes from the usage of `v8::Global`. PR-URL: #41331 Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Darshan Sen <raisinten@gmail.com>
1 parent 6187e81 commit 250e197

3 files changed

Lines changed: 30 additions & 17 deletions

File tree

‎src/env-inl.h‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ v8::Local<v8::Array> AsyncHooks::js_execution_async_resources() {
9494

9595
v8::Local<v8::Object> AsyncHooks::native_execution_async_resource(size_t i) {
9696
if (i >= native_execution_async_resources_.size()) return {};
97-
return PersistentToLocal::Strong(native_execution_async_resources_[i]);
97+
return native_execution_async_resources_[i];
9898
}
9999

100100
inline void AsyncHooks::SetJSPromiseHooks(v8::Local<v8::Function> init,
@@ -154,12 +154,11 @@ inline void AsyncHooks::push_async_context(double async_id,
154154
#endif
155155

156156
// When this call comes from JS (as a way of increasing the stack size),
157-
// `resource` will be empty, because JS caches these values anyway, and
158-
// we should avoid creating strong global references that might keep
159-
// these JS resource objects alive longer than necessary.
157+
// `resource` will be empty, because JS caches these values anyway.
160158
if (!resource.IsEmpty()) {
161159
native_execution_async_resources_.resize(offset + 1);
162-
native_execution_async_resources_[offset].Reset(env()->isolate(), resource);
160+
// Caveat: This is a v8::Local<> assignment, we do not keep a v8::Global<>!
161+
native_execution_async_resources_[offset] = resource;
163162
}
164163
}
165164

‎src/env.cc‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,20 +1096,29 @@ void AsyncHooks::Deserialize(Local<Context> context) {
10961096
async_ids_stack_.Deserialize(context);
10971097
fields_.Deserialize(context);
10981098
async_id_fields_.Deserialize(context);
1099+
1100+
Local<Array> js_execution_async_resources;
10991101
if (info_->js_execution_async_resources != 0) {
1100-
Local<Array> arr = context->GetDataFromSnapshotOnce<Array>(
1101-
info_->js_execution_async_resources)
1102-
.ToLocalChecked();
1103-
js_execution_async_resources_.Reset(context->GetIsolate(), arr);
1102+
js_execution_async_resources =
1103+
context->GetDataFromSnapshotOnce<Array>(
1104+
info_->js_execution_async_resources).ToLocalChecked();
1105+
} else {
1106+
js_execution_async_resources = Array::New(context->GetIsolate());
11041107
}
1108+
js_execution_async_resources_.Reset(
1109+
context->GetIsolate(), js_execution_async_resources);
11051110

1106-
native_execution_async_resources_.resize(
1107-
info_->native_execution_async_resources.size());
1111+
// The native_execution_async_resources_ field requires v8::Local<> instances
1112+
// for async calls whose resources were on the stack as JS objects when they
1113+
// were entered. We cannot recreate this here; however, storing these values
1114+
// on the JS equivalent gives the same result, so we do that instead.
11081115
for (size_t i = 0; i < info_->native_execution_async_resources.size(); ++i) {
1116+
if (info_->native_execution_async_resources[i] == SIZE_MAX)
1117+
continue;
11091118
Local<Object> obj = context->GetDataFromSnapshotOnce<Object>(
11101119
info_->native_execution_async_resources[i])
11111120
.ToLocalChecked();
1112-
native_execution_async_resources_[i].Reset(context->GetIsolate(), obj);
1121+
js_execution_async_resources->Set(context, i, obj).Check();
11131122
}
11141123
info_ = nullptr;
11151124
}
@@ -1155,9 +1164,11 @@ AsyncHooks::SerializeInfo AsyncHooks::Serialize(Local<Context> context,
11551164
info.native_execution_async_resources.resize(
11561165
native_execution_async_resources_.size());
11571166
for (size_t i = 0; i < native_execution_async_resources_.size(); i++) {
1158-
info.native_execution_async_resources[i] = creator->AddData(
1159-
context,
1160-
native_execution_async_resources_[i].Get(context->GetIsolate()));
1167+
info.native_execution_async_resources[i] =
1168+
native_execution_async_resources_[i].IsEmpty() ? SIZE_MAX :
1169+
creator->AddData(
1170+
context,
1171+
native_execution_async_resources_[i]);
11611172
}
11621173
CHECK_EQ(contexts_.size(), 1);
11631174
CHECK_EQ(contexts_[0], env()->context());

‎src/env.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -719,8 +719,11 @@ class AsyncHooks : public MemoryRetainer {
719719
inline void no_force_checks();
720720
inline Environment* env();
721721

722+
// NB: This call does not take (co-)ownership of `execution_async_resource`.
723+
// The lifetime of the `v8::Local<>` pointee must last until
724+
// `pop_async_context()` or `clear_async_id_stack()` are called.
722725
inline void push_async_context(double async_id, double trigger_async_id,
723-
v8::Local<v8::Object> execution_async_resource_);
726+
v8::Local<v8::Object> execution_async_resource);
724727
inline bool pop_async_context(double async_id);
725728
inline void clear_async_id_stack(); // Used in fatal exceptions.
726729

@@ -782,7 +785,7 @@ class AsyncHooks : public MemoryRetainer {
782785
void grow_async_ids_stack();
783786

784787
v8::Global<v8::Array> js_execution_async_resources_;
785-
std::vector<v8::Global<v8::Object>> native_execution_async_resources_;
788+
std::vector<v8::Local<v8::Object>> native_execution_async_resources_;
786789

787790
// Non-empty during deserialization
788791
const SerializeInfo* info_ = nullptr;

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL