Skip to content

Commit c372b99

Browse files
legendecaslegendecas
committed
lib,src: iterate module requests of a module wrap in JS
Avoid repetitively calling into JS callback from C++ in `ModuleWrap::Link`. This removes the convoluted callback style of the internal `ModuleWrap` link step.
1 parent 63d04d4 commit c372b99

File tree

6 files changed

+188
-182
lines changed

6 files changed

+188
-182
lines changed

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

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
'use strict';
22

33
const {
4+
Array,
45
ArrayPrototypeJoin,
5-
ArrayPrototypePush,
66
ArrayPrototypeSome,
77
FunctionPrototype,
88
ObjectSetPrototypeOf,
@@ -78,30 +78,8 @@ class ModuleJob {
7878
this.modulePromise = PromiseResolve(this.modulePromise);
7979
}
8080

81-
// Wait for the ModuleWrap instance being linked with all dependencies.
82-
const link = async () => {
83-
this.module = await this.modulePromise;
84-
assert(this.module instanceof ModuleWrap);
85-
86-
// Explicitly keeping track of dependency jobs is needed in order
87-
// to flatten out the dependency graph below in `_instantiate()`,
88-
// so that circular dependencies can't cause a deadlock by two of
89-
// these `link` callbacks depending on each other.
90-
const dependencyJobs = [];
91-
const promises = this.module.link(async (specifier, attributes) => {
92-
const job = await this.loader.getModuleJob(specifier, url, attributes);
93-
ArrayPrototypePush(dependencyJobs, job);
94-
return job.modulePromise;
95-
});
96-
97-
if (promises !== undefined) {
98-
await SafePromiseAllReturnVoid(promises);
99-
}
100-
101-
return SafePromiseAllReturnArrayLike(dependencyJobs);
102-
};
10381
// Promise for the list of all dependencyJobs.
104-
this.linked = link();
82+
this.linked = this._link();
10583
// This promise is awaited later anyway, so silence
10684
// 'unhandled rejection' warnings.
10785
PromisePrototypeThen(this.linked, undefined, noop);
@@ -111,6 +89,48 @@ class ModuleJob {
11189
this.instantiated = undefined;
11290
}
11391

92+
/**
93+
* Iterates the module requests and links with the loader.
94+
* @returns {Promise<ModuleJob[]>} Dependency module jobs.
95+
*/
96+
async _link() {
97+
this.module = await this.modulePromise;
98+
assert(this.module instanceof ModuleWrap);
99+
100+
const moduleRequestsLength = this.module.moduleRequests.length;
101+
// Explicitly keeping track of dependency jobs is needed in order
102+
// to flatten out the dependency graph below in `_instantiate()`,
103+
// so that circular dependencies can't cause a deadlock by two of
104+
// these `link` callbacks depending on each other.
105+
// Create an ArrayLike to avoid calling into userspace with `.then`
106+
// when returned from the async function.
107+
const dependencyJobs = Array(moduleRequestsLength);
108+
ObjectSetPrototypeOf(dependencyJobs, null);
109+
110+
// Specifiers should be aligned with the moduleRequests array in order.
111+
const specifiers = Array(moduleRequestsLength);
112+
const modulePromises = Array(moduleRequestsLength);
113+
// Iterate with index to avoid calling into userspace with `Symbol.iterator`.
114+
for (let idx = 0; idx < moduleRequestsLength; idx++) {
115+
const { specifier, attributes } = this.module.moduleRequests[idx];
116+
117+
const dependencyJobPromise = this.loader.getModuleJob(
118+
specifier, this.url, attributes,
119+
);
120+
const modulePromise = PromisePrototypeThen(dependencyJobPromise, (job) => {
121+
dependencyJobs[idx] = job;
122+
return job.modulePromise;
123+
});
124+
modulePromises[idx] = modulePromise;
125+
specifiers[idx] = specifier;
126+
}
127+
128+
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
129+
this.module.link(specifiers, modules);
130+
131+
return dependencyJobs;
132+
}
133+
114134
instantiate() {
115135
if (this.instantiated === undefined) {
116136
this.instantiated = this._instantiate();

‎lib/internal/vm/module.js‎

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,20 @@
22

33
const assert = require('internal/assert');
44
const {
5+
Array,
56
ArrayIsArray,
67
ArrayPrototypeForEach,
78
ArrayPrototypeIndexOf,
9+
ArrayPrototypeMap,
810
ArrayPrototypeSome,
911
ObjectDefineProperty,
1012
ObjectFreeze,
1113
ObjectGetPrototypeOf,
1214
ObjectSetPrototypeOf,
15+
PromiseResolve,
16+
PromisePrototypeThen,
1317
ReflectApply,
14-
SafePromiseAllReturnVoid,
18+
SafePromiseAllReturnArrayLike,
1519
Symbol,
1620
SymbolToStringTag,
1721
TypeError,
@@ -303,46 +307,62 @@ class SourceTextModule extends Module {
303307
importModuleDynamically,
304308
});
305309

306-
this[kLink] = async (linker) => {
307-
this.#statusOverride = 'linking';
310+
this[kDependencySpecifiers] = undefined;
311+
}
308312

309-
const promises = this[kWrap].link(async (identifier, attributes) => {
310-
const module = await linker(identifier, this, { attributes, assert: attributes });
311-
if (module[kWrap] === undefined) {
312-
throw new ERR_VM_MODULE_NOT_MODULE();
313-
}
314-
if (module.context !== this.context) {
315-
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
316-
}
317-
if (module.status === 'errored') {
318-
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${identifier}' resolved to an errored module`, module.error);
319-
}
320-
if (module.status === 'unlinked') {
321-
await module[kLink](linker);
322-
}
323-
return module[kWrap];
313+
async [kLink](linker) {
314+
this.#statusOverride = 'linking';
315+
316+
const moduleRequestsLength = this[kWrap].moduleRequests.length;
317+
// Iterates the module requests and links with the linker.
318+
// Specifiers should be aligned with the moduleRequests array in order.
319+
const specifiers = Array(moduleRequestsLength);
320+
const modulePromises = Array(moduleRequestsLength);
321+
// Iterates with index to avoid calling into userspace with `Symbol.iterator`.
322+
for (let idx = 0; idx < moduleRequestsLength; idx++) {
323+
const { specifier, attributes } = this[kWrap].moduleRequests[idx];
324+
325+
const linkerResult = linker(specifier, this, {
326+
attributes,
327+
assert: attributes,
324328
});
325-
326-
try {
327-
if (promises !== undefined) {
328-
await SafePromiseAllReturnVoid(promises);
329-
}
330-
} catch (e) {
331-
this.#error = e;
332-
throw e;
333-
} finally {
334-
this.#statusOverride = undefined;
335-
}
336-
};
337-
338-
this[kDependencySpecifiers] = undefined;
329+
const modulePromise = PromisePrototypeThen(
330+
PromiseResolve(linkerResult), async (module) => {
331+
if (module[kWrap] === undefined) {
332+
throw new ERR_VM_MODULE_NOT_MODULE();
333+
}
334+
if (module.context !== this.context) {
335+
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
336+
}
337+
if (module.status === 'errored') {
338+
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${specifier}' resolved to an errored module`, module.error);
339+
}
340+
if (module.status === 'unlinked') {
341+
await module[kLink](linker);
342+
}
343+
return module[kWrap];
344+
});
345+
modulePromises[idx] = modulePromise;
346+
specifiers[idx] = specifier;
347+
}
348+
349+
try {
350+
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
351+
this[kWrap].link(specifiers, modules);
352+
} catch (e) {
353+
this.#error = e;
354+
throw e;
355+
} finally {
356+
this.#statusOverride = undefined;
357+
}
339358
}
340359

341360
get dependencySpecifiers() {
342361
if (this[kWrap] === undefined) {
343362
throw new ERR_VM_MODULE_NOT_MODULE();
344363
}
345-
this[kDependencySpecifiers] ??= ObjectFreeze(this[kWrap].getStaticDependencySpecifiers());
364+
this[kDependencySpecifiers] ??= ObjectFreeze(
365+
ArrayPrototypeMap(this[kWrap].moduleRequests, (request) => request.specifier));
346366
return this[kDependencySpecifiers];
347367
}
348368

@@ -408,10 +428,10 @@ class SyntheticModule extends Module {
408428
context,
409429
identifier,
410430
});
431+
}
411432

412-
this[kLink] = () => this[kWrap].link(() => {
413-
assert.fail('link callback should not be called');
414-
});
433+
[kLink]() {
434+
/** nothing to do for synthetic modules */
415435
}
416436

417437
setExport(name, value) {

‎src/env_properties.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
V(args_string, "args") \
6868
V(asn1curve_string, "asn1Curve") \
6969
V(async_ids_stack_string, "async_ids_stack") \
70+
V(attributes_string, "attributes") \
7071
V(base_string, "base") \
7172
V(bits_string, "bits") \
7273
V(block_list_string, "blockList") \
@@ -213,6 +214,7 @@
213214
V(mgf1_hash_algorithm_string, "mgf1HashAlgorithm") \
214215
V(minttl_string, "minttl") \
215216
V(module_string, "module") \
217+
V(module_requests_string, "moduleRequests") \
216218
V(modulus_string, "modulus") \
217219
V(modulus_length_string, "modulusLength") \
218220
V(name_string, "name") \
@@ -300,6 +302,7 @@
300302
V(sni_context_string, "sni_context") \
301303
V(source_string, "source") \
302304
V(source_map_url_string, "sourceMapURL") \
305+
V(specifier_string, "specifier") \
303306
V(stack_string, "stack") \
304307
V(standard_name_string, "standardName") \
305308
V(start_time_string, "startTime") \
@@ -377,6 +380,7 @@
377380
V(js_transferable_constructor_template, v8::FunctionTemplate) \
378381
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
379382
V(message_port_constructor_template, v8::FunctionTemplate) \
383+
V(module_wrap_constructor_template, v8::FunctionTemplate) \
380384
V(microtask_queue_ctor_template, v8::FunctionTemplate) \
381385
V(pipe_constructor_template, v8::FunctionTemplate) \
382386
V(promise_wrap_template, v8::ObjectTemplate) \

0 commit comments

Comments
 (0)