Skip to content

Commit ff13d1d

Browse files
legendecastargos
legendecas
authored and
targos
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. PR-URL: #59703 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent d785929 commit ff13d1d

File tree

8 files changed

+114
-115
lines changed

8 files changed

+114
-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
@@ -393,13 +393,14 @@ class ModuleLoader {
393393
if (!job.module) {
394394
assert.fail(getRaceMessage(filename, parentFilename));
395395
}
396-
if (job.module.hasAsyncGraph) {
397-
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
398-
}
399396
const status = job.module.getStatus();
400397
debug('Module status', job, status);
398+
// hasAsyncGraph is available after module been instantiated.
399+
if (status >= kInstantiated && job.module.hasAsyncGraph) {
400+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
401+
}
401402
if (status === kEvaluated) {
402-
return { wrap: job.module, namespace: job.module.getNamespaceSync(filename, parentFilename) };
403+
return { wrap: job.module, namespace: job.module.getNamespace() };
403404
} else if (status === kInstantiated) {
404405
// When it's an async job cached by another import request,
405406
// which has finished linking but has not started its

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -324,17 +324,18 @@ 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+
this.module.link([]);
332+
}
333+
this.module.instantiate();
331334
status = this.module.getStatus();
332335
}
333336
if (status === kInstantiated || status === kErrored) {
334337
const filename = urlToFilename(this.url);
335338
const parentFilename = urlToFilename(parent?.filename);
336-
this.module.hasAsyncGraph ??= this.module.isGraphAsync();
337-
338339
if (this.module.hasAsyncGraph && !getOptionValue('--experimental-print-required-tla')) {
339340
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
340341
}
@@ -345,14 +346,19 @@ class ModuleJob extends ModuleJobBase {
345346
}
346347
throw this.module.getError();
347348
} else if (status === kEvaluating || status === kEvaluated) {
349+
if (this.module.hasAsyncGraph) {
350+
const filename = urlToFilename(this.url);
351+
const parentFilename = urlToFilename(parent?.filename);
352+
throw new ERR_REQUIRE_ASYNC_MODULE(filename, parentFilename);
353+
}
348354
// kEvaluating can show up when this is being used to deal with CJS <-> CJS cycles.
349355
// Allow it for now, since we only need to ban ESM <-> CJS cycles which would be
350356
// detected earlier during the linking phase, though the CJS handling in the ESM
351357
// loader won't be able to emit warnings on pending circular exports like what
352358
// the CJS loader does.
353359
// TODO(joyeecheung): remove the re-invented require() in the ESM loader and
354360
// always handle CJS using the CJS loader to eliminate the quirks.
355-
return { __proto__: null, module: this.module, namespace: this.module.getNamespaceSync() };
361+
return { __proto__: null, module: this.module, namespace: this.module.getNamespace() };
356362
}
357363
assert.fail(`Unexpected module status ${status}.`);
358364
}
@@ -472,7 +478,7 @@ class ModuleJobSync extends ModuleJobBase {
472478
}
473479
return { __proto__: null, module: this.module };
474480
} else if (status === kInstantiated) {
475-
// The evaluation may have been canceled because instantiateSync() detected TLA first.
481+
// The evaluation may have been canceled because instantiate() detected TLA first.
476482
// But when it is imported again, it's fine to re-evaluate it asynchronously.
477483
const timeout = -1;
478484
const breakOnSigint = false;
@@ -490,7 +496,7 @@ class ModuleJobSync extends ModuleJobBase {
490496
debug('ModuleJobSync.runSync()', this.module);
491497
assert(this.phase === kEvaluationPhase);
492498
// TODO(joyeecheung): add the error decoration logic from the async instantiate.
493-
this.module.hasAsyncGraph = this.module.instantiateSync();
499+
this.module.instantiate();
494500
// If --experimental-print-required-tla is true, proceeds to evaluation even
495501
// if it's async because we want to search for the TLA and help users locate
496502
// 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 =
@@ -687,25 +700,28 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {
687700
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
688701
Local<Context> context = obj->context();
689702
Local<Module> module = obj->module_.Get(isolate);
703+
Environment* env = realm->env();
690704

691705
if (!obj->IsLinked()) {
692-
THROW_ERR_VM_MODULE_LINK_FAILURE(realm->env(), "module is not linked");
706+
THROW_ERR_VM_MODULE_LINK_FAILURE(env, "module is not linked");
693707
return;
694708
}
695709

696-
TryCatchScope try_catch(realm->env());
697-
USE(module->InstantiateModule(
698-
context, ResolveModuleCallback, ResolveSourceCallback));
710+
{
711+
TryCatchScope try_catch(env);
712+
USE(module->InstantiateModule(
713+
context, ResolveModuleCallback, ResolveSourceCallback));
699714

700-
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
701-
CHECK(!try_catch.Message().IsEmpty());
702-
CHECK(!try_catch.Exception().IsEmpty());
703-
AppendExceptionLine(realm->env(),
704-
try_catch.Exception(),
705-
try_catch.Message(),
706-
ErrorHandlingMode::MODULE_ERROR);
707-
try_catch.ReThrow();
708-
return;
715+
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
716+
CHECK(!try_catch.Message().IsEmpty());
717+
CHECK(!try_catch.Exception().IsEmpty());
718+
AppendExceptionLine(env,
719+
try_catch.Exception(),
720+
try_catch.Message(),
721+
ErrorHandlingMode::MODULE_ERROR);
722+
try_catch.ReThrow();
723+
return;
724+
}
709725
}
710726
}
711727

@@ -790,37 +806,6 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
790806
}
791807
}
792808

793-
void ModuleWrap::InstantiateSync(const FunctionCallbackInfo<Value>& args) {
794-
Realm* realm = Realm::GetCurrent(args);
795-
Isolate* isolate = args.GetIsolate();
796-
ModuleWrap* obj;
797-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
798-
Local<Context> context = obj->context();
799-
Local<Module> module = obj->module_.Get(isolate);
800-
Environment* env = realm->env();
801-
802-
{
803-
TryCatchScope try_catch(env);
804-
USE(module->InstantiateModule(
805-
context, ResolveModuleCallback, ResolveSourceCallback));
806-
807-
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
808-
CHECK(!try_catch.Message().IsEmpty());
809-
CHECK(!try_catch.Exception().IsEmpty());
810-
AppendExceptionLine(env,
811-
try_catch.Exception(),
812-
try_catch.Message(),
813-
ErrorHandlingMode::MODULE_ERROR);
814-
try_catch.ReThrow();
815-
return;
816-
}
817-
}
818-
819-
// TODO(joyeecheung): record Module::HasTopLevelAwait() in every ModuleWrap
820-
// and infer the asynchronicity from a module's children during linking.
821-
args.GetReturnValue().Set(module->IsGraphAsync());
822-
}
823-
824809
Maybe<void> ThrowIfPromiseRejected(Realm* realm, Local<Promise> promise) {
825810
Isolate* isolate = realm->isolate();
826811
Local<Context> context = realm->context();
@@ -886,7 +871,7 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
886871
return;
887872
}
888873

889-
if (module->IsGraphAsync()) {
874+
if (obj->HasAsyncGraph()) {
890875
CHECK(env->options()->print_required_tla);
891876
auto stalled_messages =
892877
std::get<1>(module->GetStalledTopLevelAwaitMessages(isolate));
@@ -908,52 +893,15 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
908893
args.GetReturnValue().Set(module->GetModuleNamespace());
909894
}
910895

911-
void ModuleWrap::GetNamespaceSync(const FunctionCallbackInfo<Value>& args) {
912-
Realm* realm = Realm::GetCurrent(args);
913-
Isolate* isolate = args.GetIsolate();
914-
ModuleWrap* obj;
915-
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
916-
Local<Module> module = obj->module_.Get(isolate);
917-
918-
switch (module->GetStatus()) {
919-
case Module::Status::kUninstantiated:
920-
case Module::Status::kInstantiating:
921-
return realm->env()->ThrowError(
922-
"Cannot get namespace, module has not been instantiated");
923-
case Module::Status::kInstantiated:
924-
case Module::Status::kEvaluating:
925-
case Module::Status::kEvaluated:
926-
case Module::Status::kErrored:
927-
break;
928-
}
929-
930-
if (module->IsGraphAsync()) {
931-
return THROW_ERR_REQUIRE_ASYNC_MODULE(realm->env(), args[0], args[1]);
932-
}
933-
Local<Value> result = module->GetModuleNamespace();
934-
args.GetReturnValue().Set(result);
935-
}
936-
937896
void ModuleWrap::GetNamespace(const FunctionCallbackInfo<Value>& args) {
938897
Realm* realm = Realm::GetCurrent(args);
939898
Isolate* isolate = args.GetIsolate();
940899
ModuleWrap* obj;
941900
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
942901

943902
Local<Module> module = obj->module_.Get(isolate);
944-
945-
switch (module->GetStatus()) {
946-
case Module::Status::kUninstantiated:
947-
case Module::Status::kInstantiating:
948-
return realm->env()->ThrowError(
949-
"cannot get namespace, module has not been instantiated");
950-
case Module::Status::kInstantiated:
951-
case Module::Status::kEvaluating:
952-
case Module::Status::kEvaluated:
953-
case Module::Status::kErrored:
954-
break;
955-
default:
956-
UNREACHABLE();
903+
if (module->GetStatus() < Module::kInstantiated) {
904+
return THROW_ERR_MODULE_NOT_INSTANTIATED(realm->env());
957905
}
958906

959907
Local<Value> result = module->GetModuleNamespace();
@@ -1004,23 +952,28 @@ void ModuleWrap::GetStatus(const FunctionCallbackInfo<Value>& args) {
1004952
args.GetReturnValue().Set(module->GetStatus());
1005953
}
1006954

1007-
void ModuleWrap::IsGraphAsync(const FunctionCallbackInfo<Value>& args) {
955+
void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
1008956
Isolate* isolate = args.GetIsolate();
1009957
ModuleWrap* obj;
1010958
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
1011959

1012960
Local<Module> module = obj->module_.Get(isolate);
1013-
1014-
args.GetReturnValue().Set(module->IsGraphAsync());
961+
args.GetReturnValue().Set(module->GetException());
1015962
}
1016963

1017-
void ModuleWrap::GetError(const FunctionCallbackInfo<Value>& args) {
964+
void ModuleWrap::HasAsyncGraph(Local<Name> property,
965+
const PropertyCallbackInfo<Value>& args) {
1018966
Isolate* isolate = args.GetIsolate();
967+
Environment* env = Environment::GetCurrent(isolate);
1019968
ModuleWrap* obj;
1020969
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
1021970

1022971
Local<Module> module = obj->module_.Get(isolate);
1023-
args.GetReturnValue().Set(module->GetException());
972+
if (module->GetStatus() < Module::kInstantiated) {
973+
return THROW_ERR_MODULE_NOT_INSTANTIATED(env);
974+
}
975+
976+
args.GetReturnValue().Set(obj->HasAsyncGraph());
1024977
}
1025978

1026979
// static
@@ -1424,10 +1377,8 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
14241377

14251378
SetProtoMethod(isolate, tpl, "link", Link);
14261379
SetProtoMethod(isolate, tpl, "getModuleRequests", GetModuleRequests);
1427-
SetProtoMethod(isolate, tpl, "instantiateSync", InstantiateSync);
1428-
SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
1429-
SetProtoMethod(isolate, tpl, "getNamespaceSync", GetNamespaceSync);
14301380
SetProtoMethod(isolate, tpl, "instantiate", Instantiate);
1381+
SetProtoMethod(isolate, tpl, "evaluateSync", EvaluateSync);
14311382
SetProtoMethod(isolate, tpl, "evaluate", Evaluate);
14321383
SetProtoMethod(isolate, tpl, "setExport", SetSyntheticExport);
14331384
SetProtoMethod(isolate, tpl, "setModuleSourceObject", SetModuleSourceObject);
@@ -1436,9 +1387,12 @@ void ModuleWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
14361387
isolate, tpl, "createCachedData", CreateCachedData);
14371388
SetProtoMethodNoSideEffect(isolate, tpl, "getNamespace", GetNamespace);
14381389
SetProtoMethodNoSideEffect(isolate, tpl, "getStatus", GetStatus);
1439-
SetProtoMethodNoSideEffect(isolate, tpl, "isGraphAsync", IsGraphAsync);
14401390
SetProtoMethodNoSideEffect(isolate, tpl, "getError", GetError);
14411391
SetConstructorFunction(isolate, target, "ModuleWrap", tpl);
1392+
1393+
tpl->InstanceTemplate()->SetLazyDataProperty(
1394+
FIXED_ONE_BYTE_STRING(isolate, "hasAsyncGraph"), HasAsyncGraph);
1395+
14421396
isolate_data->set_module_wrap_constructor_template(tpl);
14431397

14441398
SetMethod(isolate,
@@ -1486,9 +1440,7 @@ void ModuleWrap::RegisterExternalReferences(
14861440

14871441
registry->Register(Link);
14881442
registry->Register(GetModuleRequests);
1489-
registry->Register(InstantiateSync);
14901443
registry->Register(EvaluateSync);
1491-
registry->Register(GetNamespaceSync);
14921444
registry->Register(Instantiate);
14931445
registry->Register(Evaluate);
14941446
registry->Register(SetSyntheticExport);
@@ -1498,7 +1450,7 @@ void ModuleWrap::RegisterExternalReferences(
14981450
registry->Register(GetNamespace);
14991451
registry->Register(GetStatus);
15001452
registry->Register(GetError);
1501-
registry->Register(IsGraphAsync);
1453+
registry->Register(HasAsyncGraph);
15021454

15031455
registry->Register(CreateRequiredModuleFacade);
15041456

0 commit comments

Comments
 (0)