src: reduce InternalCallbackScope overhead · nodejs/node@9696e47 · GitHub
Skip to content

Commit 9696e47

Browse files
nigrosimoneaduh95
nigrosimone
authored and
aduh95
committed
src: reduce InternalCallbackScope overhead
InternalCallbackScope looks up the Environment from the isolate two times per call, inside async_context_frame::exchange, and it keeps the prior async context frame in a v8::Global also when there is no frame, that is the common case. Every call from native code into JS pays this: MakeCallback, CallbackScope, AsyncWrap, Node-API. Now the scope passes the Environment it already has, the option is read with an inline accessor instead of copying the shared_ptr, and the global handle is created only when the prior frame is not undefined. benchmark/napi/make_callback, Node 26.3.0 built with and without this change, Linux x64, 30 runs: from 202-208 ns to 155-159 ns per call. Refs: nodejs/performance#24 Signed-off-by: Nigro Simone <nigro.simone@gmail.com> PR-URL: #66316 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 12e5405 commit 9696e47

5 files changed

Lines changed: 28 additions & 10 deletions

File tree

‎src/api/callback.cc‎

Lines changed: 7 additions & 3 deletions

‎src/async_context_frame.cc‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,23 +37,30 @@ Local<Value> current(Isolate* isolate) {
3737
return isolate->GetContinuationPreservedEmbedderDataV2().As<Value>();
3838
}
3939

40-
void set(Isolate* isolate, Local<Value> value) {
41-
auto env = Environment::GetCurrent(isolate);
42-
if (!env->options()->async_context_frame) {
40+
void set(Environment* env, Local<Value> value) {
41+
if (!env->async_context_frame_enabled()) {
4342
return;
4443
}
4544

46-
isolate->SetContinuationPreservedEmbedderDataV2(value);
45+
env->isolate()->SetContinuationPreservedEmbedderDataV2(value);
46+
}
47+
48+
void set(Isolate* isolate, Local<Value> value) {
49+
set(Environment::GetCurrent(isolate), value);
4750
}
4851

4952
// NOTE: It's generally recommended to use async_context_frame::Scope
5053
// but sometimes (such as enterWith) a direct exchange is needed.
51-
Local<Value> exchange(Isolate* isolate, Local<Value> value) {
52-
auto prior = current(isolate);
53-
set(isolate, value);
54+
Local<Value> exchange(Environment* env, Local<Value> value) {
55+
auto prior = current(env->isolate());
56+
set(env, value);
5457
return prior;
5558
}
5659

60+
Local<Value> exchange(Isolate* isolate, Local<Value> value) {
61+
return exchange(Environment::GetCurrent(isolate), value);
62+
}
63+
5764
void CreatePerContextProperties(Local<Object> target,
5865
Local<Value> unused,
5966
Local<Context> context,

‎src/async_context_frame.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ class Scope {
2323

2424
v8::Local<v8::Value> current(v8::Isolate* isolate);
2525
void set(v8::Isolate* isolate, v8::Local<v8::Value> value);
26+
void set(Environment* env, v8::Local<v8::Value> value);
2627
v8::Local<v8::Value> exchange(v8::Isolate* isolate, v8::Local<v8::Value> value);
28+
v8::Local<v8::Value> exchange(Environment* env, v8::Local<v8::Value> value);
2729

2830
} // namespace async_context_frame
2931
} // namespace node

‎src/env-inl.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,10 @@ inline std::shared_ptr<EnvironmentOptions> Environment::options() {
460460
return options_;
461461
}
462462

463+
inline bool Environment::async_context_frame_enabled() const {
464+
return options_->async_context_frame;
465+
}
466+
463467
inline const std::vector<std::string>& Environment::argv() {
464468
return argv_;
465469
}

‎src/env.h‎

Lines changed: 1 addition & 0 deletions

0 commit comments

Comments
 (0)