Skip to content

Commit 8cf0478

Browse files
committed
lib,src: cache ModuleWrap.hasAsyncGraph
`v8::Module::IsGraphAsync()` traverses the dependencies to find if they contain TLA each time. `ModuleWrap.hasAsyncGraph` caches the result and exposes the property to JS land so that the presence of the property `module.hasAsyncGraph` can be consistent. This also allows C++ access of cached `hasAsyncGraph`. This merges the `intantiateSync`/`instantiate` and `getNamespaceSync`/`getNamespace` as they are always sync.
1 parent 910c879 commit 8cf0478

File tree

8 files changed

+117
-115
lines changed

8 files changed

+117
-115
lines changed

lib/internal/bootstrap/realm.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,6 @@ class BuiltinModule {
359359
this.setExport('default', builtin.exports);
360360
});
361361
// Ensure immediate sync execution to capture exports now
362-
this.module.link([]);
363362
this.module.instantiate();
364363
this.module.evaluate(-1, false);
365364
return this.module;

lib/internal/modules/esm/loader.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,13 +387,14 @@ class ModuleLoader {
387387
if (!job.module) {
388388
assert.fail(getRaceMessage(filename, parentFilename));
389389
}
390-
if (job.module.hasAsyncGraph) {
391-
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
392-
}
393390
const status = job.module.getStatus();
394391
debug('Module status', job, status);
392+
// hasAsyncGraph is available after module been instantiated.
393+
if (status >= kInstantiated && job.module.hasAsyncGraph) {
394+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
395+
}
395396
if (status === kEvaluated) {
396-
return { wrap: job.module, namespace: job.module.getNamespaceSync(filename, parentFilename) };
397+
return { wrap: job.module, namespace: job.module.getNamespace() };
397398
} else if (status === kInstantiated) {
398399
// When it's an async job cached by another import request,
399400
// which has finished linking but has not started its

lib/internal/modules/esm/module_job.js

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -324,17 +324,21 @@ class ModuleJob extends ModuleJobBase {
324324
let status = this.module.getStatus();
325325

326326
debug('ModuleJob.runSync()', status, this.module);
327-
// FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
328-
// fully synchronous instead.
329327
if (status === kUninstantiated) {
330-
this.module.hasAsyncGraph = this.module.instantiateSync();
328+
// FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
329+
// fully synchronous instead.
330+
if (this.module.getModuleRequests().length > 0) {
331+
const filename = urlToFilename(this.url);
332+
const parentFilename = urlToFilename(parent?.filename);
333+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
334+
}
335+
this.module.link([]);
336+
this.module.instantiate();
331337
status = this.module.getStatus();
332338
}
333339
if (status === kInstantiated || status === kErrored) {
334340
const filename = urlToFilename(this.url);
335341
const parentFilename = urlToFilename(parent?.filename);
336-
this.module.hasAsyncGraph ??= this.module.isGraphAsync();
337-
338342
if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
339343
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
340344
}
@@ -345,14 +349,19 @@ class ModuleJob extends ModuleJobBase {
345349
}
346350
throw this.module.getError();
347351
} else if (status === kEvaluating || status === kEvaluated) {
352+
if (this.module.hasAsyncGraph) {
353+
const filename = urlToFilename(this.url);
354+
const parentFilename = urlToFilename(parent?.filename);
355+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
356+
}
348357
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
349358
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
350359
// detected earlier during the linking phase, though the CJS handling in the ESM
351360
// loader won't be able to emit warnings on pending circular exports like what
352361
// the CJS loader does.
353362
// TODO(joyeecheung): remove the re-invented require() in the ESM loader and
354363
// always handle CJS using the CJS loader to eliminate the quirks.
355-
return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
364+
return { __proto__: null, module: this.module, namespace: this.module.getNamespace() };
356365
}
357366
assert.fail(`Unexpected module status ${status}.`);
358367
}
@@ -472,7 +481,7 @@ class ModuleJobSync extends ModuleJobBase {
472481
}
473482
return { __proto__: null, module: this.module };
474483
} else if (status === kInstantiated) {
475-
// The evaluation may have been canceled because instantiateSync() detected TLA first.
484+
// The evaluation may have been canceled because instantiate() detected TLA first.
476485
// But when it is imported again, it's fine to re-evaluate it asynchronously.
477486
const timeout = -1;
478487
const breakOnSigint = false;
@@ -490,7 +499,7 @@ class ModuleJobSync extends ModuleJobBase {
490499
debug('ModuleJobSync.runSync()', this.module);
491500
assert(this.phase === kEvaluationPhase);
492501
// TODO(joyeecheung): add the error decoration logic from the async instantiate.
493-
this.module.hasAsyncGraph = this.module.instantiateSync();
502+
this.module.instantiate();
494503
// If --experimental-print-required-tla is true, proceeds to evaluation even
495504
// if it's async because we want to search for the TLA and help users locate
496505
// them.

lib/internal/vm/module.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,6 @@ class SyntheticModule extends Module {
469469
identifier,
470470
});
471471
// A synthetic module does not have dependencies.
472-
this[kWrap].link([]);
473472
this[kWrap].instantiate();
474473
}
475474

src/module_wrap.cc

Lines changed: 49 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ using v8::ObjectTemplate;
5656
using v8::PrimitiveArray;
5757
using v8::Promise;
5858
using v8::PromiseRejectEvent;
59+
using v8::PropertyCallbackInfo;
5960
using v8::ScriptCompiler;
6061
using v8::ScriptOrigin;
6162
using v8::String;
@@ -158,6 +159,8 @@ ModuleWrap::ModuleWrap(Realm* realm,
158159

159160
if (!synthetic_evaluation_step->IsUndefined()) {
160161
synthetic_ = true;
162+
// Synthetic modules have no dependencies.
163+
linked_ = true;
161164
}
162165
MakeWeak();
163166
module_.SetWeak();
@@ -240,7 +243,7 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
240243
return Just(true);
241244
}
242245

243-
if (!module->IsGraphAsync()) { // There is no TLA, no need to check.
246+
if (!HasAsyncGraph()) { // There is no TLA, no need to check.
244247
return Just(true);
245248
}
246249

@@ -263,6 +266,16 @@ Maybe<bool> ModuleWrap::CheckUnsettledTopLevelAwait() {
263266
return Just(false);
264267
}
265268

269+
bool ModuleWrap::HasAsyncGraph() {
270+
if (!has_async_graph_.has_value()) {
271+
Isolate* isolate = env()->isolate();
272+
HandleScope scope(isolate);
273+
274+
has_async_graph_ = module_.Get(isolate)->IsGraphAsync();
275+
}
276+
return has_async_graph_.value();
277+
}
278+
266279
Local<PrimitiveArray> ModuleWrap::GetHostDefinedOptions(
267280
Isolate* isolate, Local<Symbol> id_symbol) {
268281
Local<PrimitiveArray> host_defined_options =
@@ -680,25 +693,28 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
680693
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
681694
Local<Context> context = obj->context();
682695
Local<Module> module = obj->module_.Get(isolate);
696+
Environment* env = realm->env();
683697

684698
if (!obj->IsLinked()) {
685-
THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is not linked");
699+
THROW_ERR_VM_MODULE_LINK_FAILURE(env, "module is not linked");
686700
return;
687701
}
688702

689-
TryCatchScope try_catch(realm->env());
690-
USE(module->InstantiateModule(
691-
context, ResolveModuleCallback, ResolveSourceCallback));
703+
{
704+
TryCatchScope try_catch(env);
705+
USE(module->InstantiateModule(
706+
context, ResolveModuleCallback, ResolveSourceCallback));
692707

693-
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
694-
CHECK(!try_catch.Message().IsEmpty());
695-
CHECK(!try_catch.Exception().IsEmpty());
696-
AppendExceptionLine(realm->env(),
697-
try_catch.Exception(),
698-
try_catch.Message(),
699-
ErrorHandlingMode::MODULE_ERROR);
700-
try_catch.ReThrow();
701-
return;
708+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
709+
CHECK(!try_catch.Message().IsEmpty());
710+
CHECK(!try_catch.Exception().IsEmpty());
711+
AppendExceptionLine(env,
712+
try_catch.Exception(),
713+
try_catch.Message(),
714+
ErrorHandlingMode::MODULE_ERROR);
715+
try_catch.ReThrow();
716+
return;
717+
}
702718
}
703719
}
704720

@@ -783,37 +799,6 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
783799
}
784800
}
785801

786-
void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
787-
Realm* realm = Realm::GetCurrent(args);
788-
Isolate* isolate = args.GetIsolate();
789-
ModuleWrap* obj;
790-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
791-
Local<Context> context = obj->context();
792-
Local<Module> module = obj->module_.Get(isolate);
793-
Environment* env = realm->env();
794-
795-
{
796-
TryCatchScope try_catch(env);
797-
USE(module->InstantiateModule(
798-
context, ResolveModuleCallback, ResolveSourceCallback));
799-
800-
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
801-
CHECK(!try_catch.Message().IsEmpty());
802-
CHECK(!try_catch.Exception().IsEmpty());
803-
AppendExceptionLine(env,
804-
try_catch.Exception(),
805-
try_catch.Message(),
806-
ErrorHandlingMode::MODULE_ERROR);
807-
try_catch.ReThrow();
808-
return;
809-
}
810-
}
811-
812-
// TODO(joyeecheung): record Module::HasTopLevelAwait() in every ModuleWrap
813-
// and infer the asynchronicity from a module's children during linking.
814-
args.GetReturnValue().Set(module->IsGraphAsync());
815-
}
816-
817802
Maybe<void> ThrowIfPromiseRejected(Realm* realm, Local<Promise> promise) {
818803
Isolate* isolate = realm->isolate();
819804
Local<Context> context = realm->context();
@@ -879,7 +864,7 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
879864
return;
880865
}
881866

882-
if (module->IsGraphAsync()) {
867+
if (obj->HasAsyncGraph()) {
883868
CHECK(env->options()->print_required_tla);
884869
auto stalled_messages =
885870
std::get<1>(module->GetStalledTopLevelAwaitMessages(isolate));
@@ -901,52 +886,15 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
901886
args.GetReturnValue().Set(module->GetModuleNamespace());
902887
}
903888

904-
void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
905-
Realm* realm = Realm::GetCurrent(args);
906-
Isolate* isolate = args.GetIsolate();
907-
ModuleWrap* obj;
908-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
909-
Local<Module> module = obj->module_.Get(isolate);
910-
911-
switch (module->GetStatus()) {
912-
case Module::Status::kUninstantiated:
913-
case Module::Status::kInstantiating:
914-
return realm->env()->ThrowError(
915-
"Cannot get namespace, module has not been instantiated");
916-
case Module::Status::kInstantiated:
917-
case Module::Status::kEvaluating:
918-
case Module::Status::kEvaluated:
919-
case Module::Status::kErrored:
920-
break;
921-
}
922-
923-
if (module->IsGraphAsync()) {
924-
return THROW_ERR_REQUIRE_ASYNC_MODULE(realm->env(), args[0], args[1]);
925-
}
926-
Local<Value> result = module->GetModuleNamespace();
927-
args.GetReturnValue().Set(result);
928-
}
929-
930889
void ModuleWrap::GetNamespace(const FunctionCallbackInfo<Value>& args) {
931890
Realm* realm = Realm::GetCurrent(args);
932891
Isolate* isolate = args.GetIsolate();
933892
ModuleWrap* obj;
934893
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
935894

936895
Local<Module> module = obj->module_.Get(isolate);
937-
938-
switch (module->GetStatus()) {
939-
case Module::Status::kUninstantiated:
940-
case Module::Status::kInstantiating:
941-
return realm->env()->ThrowError(
942-
"cannot get namespace, module has not been instantiated");
943-
case Module::Status::kInstantiated:
944-
case Module::Status::kEvaluating:
945-
case Module::Status::kEvaluated:
946-
case Module::Status::kErrored:
947-
break;
948-
default:
949-
UNREACHABLE();
896+
if (module->GetStatus() < Module::kInstantiated) {
897+
return THROW_ERR_MODULE_NOT_INSTANTIATED(realm->env());
950898
}
951899

952900
Local<Value> result = module->GetModuleNamespace();
@@ -997,23 +945,28 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
997945
args.GetReturnValue().Set(module->GetStatus());
998946
}
999947

1000-
void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo<Value>& args) {
948+
void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
1001949
Isolate* isolate = args.GetIsolate();
1002950
ModuleWrap* obj;
1003951
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
1004952

1005953
Local<Module> module = obj->module_.Get(isolate);
1006-
1007-
args.GetReturnValue().Set(module->IsGraphAsync());
954+
args.GetReturnValue().Set(module->GetException());
1008955
}
1009956

1010-
void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
957+
void ModuleWrap::HasAsyncGraph(Local<Name> property,
958+
const PropertyCallbackInfo<Value>& args) {
1011959
Isolate* isolate = args.GetIsolate();
960+
Environment* env = Environment::GetCurrent(isolate);
1012961
ModuleWrap* obj;
1013962
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
1014963

1015964
Local<Module> module = obj->module_.Get(isolate);
1016-
args.GetReturnValue().Set(module->GetException());
965+
if (module->GetStatus() < Module::kInstantiated) {
966+
return THROW_ERR_MODULE_NOT_INSTANTIATED(env);
967+
}
968+
969+
args.GetReturnValue().Set(obj->HasAsyncGraph());
1017970
}
1018971

1019972
// static
@@ -1417,10 +1370,8 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
14171370

14181371
SetProtoMethod(isolate, tpl, "link", Link);
14191372
SetProtoMethod(isolate, tpl, "getModuleRequests", GetModuleRequests);
1420-
SetProtoMethod(isolate, tpl, "instantiateSync", InstantiateSync);
1421-
SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
1422-
SetProtoMethod(isolate, tpl, "getNamespaceSync", GetNamespaceSync);
14231373
SetProtoMethod(isolate, tpl, "instantiate", Instantiate);
1374+
SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
14241375
SetProtoMethod(isolate, tpl, "evaluate", Evaluate);
14251376
SetProtoMethod(isolate, tpl, "setExport", SetSyntheticExport);
14261377
SetProtoMethod(isolate, tpl, "setModuleSourceObject", SetModuleSourceObject);
@@ -1429,9 +1380,12 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
14291380
isolate, tpl, "createCachedData", CreateCachedData);
14301381
SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace);
14311382
SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus);
1432-
SetProtoMethodNoSideEffect(isolate, tpl, "isGraphAsync", IsGraphAsync);
14331383
SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError);
14341384
SetConstructorFunction(isolate, target, "ModuleWrap", tpl);
1385+
1386+
tpl->InstanceTemplate()->SetLazyDataProperty(
1387+
FIXED_ONE_BYTE_STRING(isolate, "hasAsyncGraph"), HasAsyncGraph);
1388+
14351389
isolate_data->set_module_wrap_constructor_template(tpl);
14361390

14371391
SetMethod(isolate,
@@ -1479,9 +1433,7 @@ void ModuleWrap::RegisterExternalReferences(
14791433

14801434
registry->Register(Link);
14811435
registry->Register(GetModuleRequests);
1482-
registry->Register(InstantiateSync);
14831436
registry->Register(EvaluateSync);
1484-
registry->Register(GetNamespaceSync);
14851437
registry->Register(Instantiate);
14861438
registry->Register(Evaluate);
14871439
registry->Register(SetSyntheticExport);
@@ -1491,7 +1443,7 @@ void ModuleWrap::RegisterExternalReferences(
14911443
registry->Register(GetNamespace);
14921444
registry->Register(GetStatus);
14931445
registry->Register(GetError);
1494-
registry->Register(IsGraphAsync);
1446+
registry->Register(HasAsyncGraph);
14951447

14961448
registry->Register(CreateRequiredModuleFacade);
14971449

0 commit comments

Comments
 (0)