Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 250e197

Browse filesBrowse files
addaleaxtargos
authored andcommitted
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
Copy full SHA for 250e197

File tree

Expand file treeCollapse file tree

3 files changed

+30
-17
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+30
-17
lines changed
Open diff view settings
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+4-5Lines 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

Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+21-10Lines 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());
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+5-2Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.