Skip to content

Commit 3e40d87

Browse files
ptomatoedusperoni
authored andcommitted
chore: Don't store Global<Context> in callback info
Callbacks (runOnMainThread, frame, and timer) can run in the context in which they were created, which we can get via GetCreationContext(). So, there is no need to store a v8::Global<v8::Context> in which to invoke the callback. See v8/v8@b38bf5b0 for why it's OK to use ToLocalChecked() - the creation context is only null if the objects were created in WASM instead of JS.
1 parent 016041e commit 3e40d87

File tree

4 files changed

+15
-25
lines changed

4 files changed

+15
-25
lines changed

test-app/runtime/src/main/cpp/CallbackHandlers.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -667,13 +667,11 @@ void CallbackHandlers::RunOnMainThreadCallback(const FunctionCallbackInfo<v8::Va
667667
Isolate::Scope isolate_scope(isolate);
668668
HandleScope handle_scope(isolate);
669669

670-
Local<Context> context = isolate->GetCurrentContext();
671-
672670
uint64_t key = ++count_;
673671
Local<v8::Function> callback = args[0].As<v8::Function>();
674672

675673
bool inserted;
676-
std::tie(std::ignore, inserted) = cache_.try_emplace(key, isolate, callback, context);
674+
std::tie(std::ignore, inserted) = cache_.try_emplace(key, isolate, callback);
677675
assert(inserted && "Main thread callback ID should not be duplicated");
678676

679677
auto value = Callback(key);
@@ -697,9 +695,9 @@ int CallbackHandlers::RunOnMainThreadFdCallback(int fd, int events, void *data)
697695
v8::Locker locker(isolate);
698696
Isolate::Scope isolate_scope(isolate);
699697
HandleScope handle_scope(isolate);
700-
auto context = it->second.context_.Get(isolate);
701-
Context::Scope context_scope(context);
702698
Local<v8::Function> cb = it->second.callback_.Get(isolate);
699+
v8::Local<v8::Context> context = cb->GetCreationContextChecked();
700+
Context::Scope context_scope(context);
703701

704702
v8::TryCatch tc(isolate);
705703

@@ -1655,7 +1653,7 @@ void CallbackHandlers::PostFrameCallback(const FunctionCallbackInfo<v8::Value> &
16551653

16561654
robin_hood::unordered_map<uint64_t, FrameCallbackCacheEntry>::iterator val;
16571655
bool inserted;
1658-
std::tie(val, inserted) = frameCallbackCache_.try_emplace(key, isolate, callback, context, key);
1656+
std::tie(val, inserted) = frameCallbackCache_.try_emplace(key, isolate, callback, key);
16591657
assert(inserted && "Frame callback ID should not be duplicated");
16601658

16611659
PostCallback(args, &val->second, context);
@@ -1757,4 +1755,4 @@ jmethodID CallbackHandlers::INIT_WORKER_METHOD_ID = nullptr;
17571755

17581756
NumericCasts CallbackHandlers::castFunctions;
17591757
ArrayElementAccessor CallbackHandlers::arrayElementAccessor;
1760-
FieldAccessor CallbackHandlers::fieldAccessor;
1758+
FieldAccessor CallbackHandlers::fieldAccessor;

test-app/runtime/src/main/cpp/CallbackHandlers.h

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -297,20 +297,17 @@ namespace tns {
297297

298298

299299
struct CacheEntry {
300-
CacheEntry(v8::Isolate* isolate, v8::Local<v8::Function> callback, v8::Local<v8::Context> context)
300+
CacheEntry(v8::Isolate* isolate, v8::Local<v8::Function> callback)
301301
: isolate_(isolate),
302-
callback_(isolate, callback),
303-
context_(isolate, context){
302+
callback_(isolate, callback) {
304303
}
305304

306305
~CacheEntry() {
307-
context_.Reset();
308306
callback_.Reset();
309307
}
310308

311309
v8::Isolate* isolate_;
312310
v8::Global<v8::Function> callback_;
313-
v8::Global<v8::Context> context_;
314311
};
315312

316313
static robin_hood::unordered_map<uint64_t, CacheEntry> cache_;
@@ -319,22 +316,18 @@ namespace tns {
319316
static std::atomic_uint64_t frameCallbackCount_;
320317

321318
struct FrameCallbackCacheEntry {
322-
FrameCallbackCacheEntry(v8::Isolate *isolate, v8::Local<v8::Function> callback,
323-
v8::Local<v8::Context> context, uint64_t aId)
319+
FrameCallbackCacheEntry(v8::Isolate *isolate, v8::Local<v8::Function> callback, uint64_t aId)
324320
: isolate_(isolate),
325321
callback_(isolate, callback),
326-
context_(isolate, context),
327322
id(aId) {
328323
}
329324

330325
~FrameCallbackCacheEntry() {
331-
context_.Reset();
332326
callback_.Reset();
333327
}
334328

335329
v8::Isolate *isolate_;
336330
v8::Global<v8::Function> callback_;
337-
v8::Global<v8::Context> context_;
338331
bool removed = false;
339332
uint64_t id;
340333

@@ -358,10 +351,10 @@ namespace tns {
358351
v8::Locker locker(isolate);
359352
v8::Isolate::Scope isolate_scope(isolate);
360353
v8::HandleScope handle_scope(isolate);
361-
auto context = entry->context_.Get(isolate);
354+
v8::Local<v8::Function> cb = entry->callback_.Get(isolate);
355+
v8::Local<v8::Context> context = cb->GetCreationContextChecked();
362356
v8::Context::Scope context_scope(context);
363357

364-
v8::Local<v8::Function> cb = entry->callback_.Get(isolate);
365358
v8::Local<v8::Value> args[1] = {v8::Number::New(isolate, ts)};
366359

367360
v8::TryCatch tc(isolate);

test-app/runtime/src/main/cpp/Timers.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,8 @@ int Timers::PumpTimerLoopCallback(int fd, int events, void *data) {
293293
task->startTime_ = task->dueTime_;
294294
thiz->addTask(task);
295295
}
296-
auto context = task->context_.Get(isolate);
296+
v8::Local<v8::Function> cb = task->callback_.Get(isolate);
297+
v8::Local<v8::Context> context = cb->GetCreationContextChecked();
297298
Context::Scope context_scope(context);
298299
TryCatch tc(isolate);
299300
auto argc = task->args_.get() == nullptr ? 0 : task->args_->size();
@@ -302,9 +303,9 @@ int Timers::PumpTimerLoopCallback(int fd, int events, void *data) {
302303
for (int i = 0; i < argc; i++) {
303304
argv[i] = task->args_->at(i)->Get(isolate);
304305
}
305-
task->callback_.Get(isolate)->Call(context, context->Global(), argc, argv);
306+
cb->Call(context, context->Global(), argc, argv);
306307
} else {
307-
task->callback_.Get(isolate)->Call(context, context->Global(), 0, nullptr);
308+
cb->Call(context, context->Global(), 0, nullptr);
308309
}
309310
// task is not queued, so it's either a setTimeout or a cleared setInterval
310311
// ensure we remove it

test-app/runtime/src/main/cpp/Timers.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace tns {
2020
bool repeats,
2121
const std::shared_ptr<std::vector<std::shared_ptr<v8::Persistent<v8::Value>>>> &args,
2222
int id, double startTime)
23-
: isolate_(isolate), context_(isolate, context), callback_(isolate, callback),
23+
: isolate_(isolate), callback_(isolate, callback),
2424
frequency_(frequency), repeats_(repeats), args_(args), id_(id),
2525
startTime_(startTime) {
2626

@@ -36,7 +36,6 @@ namespace tns {
3636
}
3737

3838
inline void Unschedule() {
39-
context_.Reset();
4039
callback_.Reset();
4140
args_.reset();
4241
isolate_ = nullptr;
@@ -46,7 +45,6 @@ namespace tns {
4645

4746
int nestingLevel_ = 0;
4847
v8::Isolate *isolate_;
49-
v8::Persistent<v8::Context> context_;
5048
v8::Persistent<v8::Function> callback_;
5149
std::shared_ptr<std::vector<std::shared_ptr<v8::Persistent<v8::Value>>>> args_;
5250
bool repeats_ = false;

0 commit comments

Comments
 (0)