Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f13589f

Browse filesBrowse files
legendecasmarco-ippolito
legendecas
authored and
marco-ippolito
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. PR-URL: #52058 Backport-PR-URL: #56927 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Refs: #52697
1 parent b07ad39 commit f13589f
Copy full SHA for f13589f

File tree

Expand file treeCollapse file tree

9 files changed

+219
-229
lines changed
Filter options
Expand file treeCollapse file tree

9 files changed

+219
-229
lines changed

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

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ class ModuleLoader {
318318
* @param {object} importAttributes import attributes from the import statement.
319319
* @returns {ModuleJobBase}
320320
*/
321-
getModuleWrapForRequire(specifier, parentURL, importAttributes) {
321+
getModuleJobForRequire(specifier, parentURL, importAttributes) {
322322
assert(getOptionValue('--experimental-require-module'));
323323

324324
if (canParse(specifier)) {

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

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/module_job.js
+57-35Lines changed: 57 additions & 35 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,
@@ -82,31 +82,8 @@ class ModuleJob extends ModuleJobBase {
8282
this.modulePromise = PromiseResolve(this.modulePromise);
8383
}
8484

85-
// Wait for the ModuleWrap instance being linked with all dependencies.
86-
const link = async () => {
87-
this.module = await this.modulePromise;
88-
assert(this.module instanceof ModuleWrap);
89-
90-
// Explicitly keeping track of dependency jobs is needed in order
91-
// to flatten out the dependency graph below in `_instantiate()`,
92-
// so that circular dependencies can't cause a deadlock by two of
93-
// these `link` callbacks depending on each other.
94-
const dependencyJobs = [];
95-
const promises = this.module.link(async (specifier, attributes) => {
96-
const job = await this.#loader.getModuleJob(specifier, url, attributes);
97-
debug(`async link() ${this.url} -> ${specifier}`, job);
98-
ArrayPrototypePush(dependencyJobs, job);
99-
return job.modulePromise;
100-
});
101-
102-
if (promises !== undefined) {
103-
await SafePromiseAllReturnVoid(promises);
104-
}
105-
106-
return SafePromiseAllReturnArrayLike(dependencyJobs);
107-
};
10885
// Promise for the list of all dependencyJobs.
109-
this.linked = link();
86+
this.linked = this._link();
11087
// This promise is awaited later anyway, so silence
11188
// 'unhandled rejection' warnings.
11289
PromisePrototypeThen(this.linked, undefined, noop);
@@ -116,6 +93,49 @@ class ModuleJob extends ModuleJobBase {
11693
this.instantiated = undefined;
11794
}
11895

96+
/**
97+
* Iterates the module requests and links with the loader.
98+
* @returns {Promise<ModuleJob[]>} Dependency module jobs.
99+
*/
100+
async _link() {
101+
this.module = await this.modulePromise;
102+
assert(this.module instanceof ModuleWrap);
103+
104+
const moduleRequests = this.module.getModuleRequests();
105+
// Explicitly keeping track of dependency jobs is needed in order
106+
// to flatten out the dependency graph below in `_instantiate()`,
107+
// so that circular dependencies can't cause a deadlock by two of
108+
// these `link` callbacks depending on each other.
109+
// Create an ArrayLike to avoid calling into userspace with `.then`
110+
// when returned from the async function.
111+
const dependencyJobs = Array(moduleRequests.length);
112+
ObjectSetPrototypeOf(dependencyJobs, null);
113+
114+
// Specifiers should be aligned with the moduleRequests array in order.
115+
const specifiers = Array(moduleRequests.length);
116+
const modulePromises = Array(moduleRequests.length);
117+
// Iterate with index to avoid calling into userspace with `Symbol.iterator`.
118+
for (let idx = 0; idx < moduleRequests.length; idx++) {
119+
const { specifier, attributes } = moduleRequests[idx];
120+
121+
const dependencyJobPromise = this.#loader.getModuleJob(
122+
specifier, this.url, attributes,
123+
);
124+
const modulePromise = PromisePrototypeThen(dependencyJobPromise, (job) => {
125+
debug(`async link() ${this.url} -> ${specifier}`, job);
126+
dependencyJobs[idx] = job;
127+
return job.modulePromise;
128+
});
129+
modulePromises[idx] = modulePromise;
130+
specifiers[idx] = specifier;
131+
}
132+
133+
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
134+
this.module.link(specifiers, modules);
135+
136+
return dependencyJobs;
137+
}
138+
119139
instantiate() {
120140
if (this.instantiated === undefined) {
121141
this.instantiated = this._instantiate();
@@ -269,18 +289,20 @@ class ModuleJobSync extends ModuleJobBase {
269289
super(url, importAttributes, moduleWrap, isMain, inspectBrk, true);
270290
assert(this.module instanceof ModuleWrap);
271291
this.#loader = loader;
272-
const moduleRequests = this.module.getModuleRequestsSync();
273-
const linked = [];
292+
const moduleRequests = this.module.getModuleRequests();
293+
// Specifiers should be aligned with the moduleRequests array in order.
294+
const specifiers = Array(moduleRequests.length);
295+
const modules = Array(moduleRequests.length);
296+
const jobs = Array(moduleRequests.length);
274297
for (let i = 0; i < moduleRequests.length; ++i) {
275-
const { 0: specifier, 1: attributes } = moduleRequests[i];
276-
const job = this.#loader.getModuleWrapForRequire(specifier, url, attributes);
277-
const isLast = (i === moduleRequests.length - 1);
278-
// TODO(joyeecheung): make the resolution callback deal with both promisified
279-
// an raw module wraps, then we don't need to wrap it with a promise here.
280-
this.module.cacheResolvedWrapsSync(specifier, PromiseResolve(job.module), isLast);
281-
ArrayPrototypePush(linked, job);
298+
const { specifier, attributes } = moduleRequests[i];
299+
const job = this.#loader.getModuleJobForRequire(specifier, url, attributes);
300+
specifiers[i] = specifier;
301+
modules[i] = job.module;
302+
jobs[i] = job;
282303
}
283-
this.linked = linked;
304+
this.module.link(specifiers, modules);
305+
this.linked = jobs;
284306
}
285307

286308
get modulePromise() {

‎lib/internal/vm/module.js

Copy file name to clipboardExpand all lines: lib/internal/vm/module.js
+56-35Lines changed: 56 additions & 35 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
ObjectGetPrototypeOf,
1113
ObjectPrototypeHasOwnProperty,
1214
ObjectSetPrototypeOf,
15+
PromiseResolve,
16+
PromisePrototypeThen,
1317
ReflectApply,
14-
SafePromiseAllReturnVoid,
18+
SafePromiseAllReturnArrayLike,
1519
Symbol,
1620
SymbolToStringTag,
1721
TypeError,
@@ -293,44 +297,61 @@ class SourceTextModule extends Module {
293297
importModuleDynamically,
294298
});
295299

296-
this[kLink] = async (linker) => {
297-
this.#statusOverride = 'linking';
300+
this[kDependencySpecifiers] = undefined;
301+
}
298302

299-
const promises = this[kWrap].link(async (identifier, attributes) => {
300-
const module = await linker(identifier, this, { attributes, assert: attributes });
301-
if (!isModule(module)) {
302-
throw new ERR_VM_MODULE_NOT_MODULE();
303-
}
304-
if (module.context !== this.context) {
305-
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
306-
}
307-
if (module.status === 'errored') {
308-
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${identifier}' resolved to an errored module`, module.error);
309-
}
310-
if (module.status === 'unlinked') {
311-
await module[kLink](linker);
312-
}
313-
return module[kWrap];
303+
async [kLink](linker) {
304+
this.#statusOverride = 'linking';
305+
306+
const moduleRequests = this[kWrap].getModuleRequests();
307+
// Iterates the module requests and links with the linker.
308+
// Specifiers should be aligned with the moduleRequests array in order.
309+
const specifiers = Array(moduleRequests.length);
310+
const modulePromises = Array(moduleRequests.length);
311+
// Iterates with index to avoid calling into userspace with `Symbol.iterator`.
312+
for (let idx = 0; idx < moduleRequests.length; idx++) {
313+
const { specifier, attributes } = moduleRequests[idx];
314+
315+
const linkerResult = linker(specifier, this, {
316+
attributes,
317+
assert: attributes,
314318
});
319+
const modulePromise = PromisePrototypeThen(
320+
PromiseResolve(linkerResult), async (module) => {
321+
if (!isModule(module)) {
322+
throw new ERR_VM_MODULE_NOT_MODULE();
323+
}
324+
if (module.context !== this.context) {
325+
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
326+
}
327+
if (module.status === 'errored') {
328+
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${specifier}' resolved to an errored module`, module.error);
329+
}
330+
if (module.status === 'unlinked') {
331+
await module[kLink](linker);
332+
}
333+
return module[kWrap];
334+
});
335+
modulePromises[idx] = modulePromise;
336+
specifiers[idx] = specifier;
337+
}
315338

316-
try {
317-
if (promises !== undefined) {
318-
await SafePromiseAllReturnVoid(promises);
319-
}
320-
} catch (e) {
321-
this.#error = e;
322-
throw e;
323-
} finally {
324-
this.#statusOverride = undefined;
325-
}
326-
};
327-
328-
this[kDependencySpecifiers] = undefined;
339+
try {
340+
const modules = await SafePromiseAllReturnArrayLike(modulePromises);
341+
this[kWrap].link(specifiers, modules);
342+
} catch (e) {
343+
this.#error = e;
344+
throw e;
345+
} finally {
346+
this.#statusOverride = undefined;
347+
}
329348
}
330349

331350
get dependencySpecifiers() {
332351
validateInternalField(this, kDependencySpecifiers, 'SourceTextModule');
333-
this[kDependencySpecifiers] ??= this[kWrap].getStaticDependencySpecifiers();
352+
// TODO(legendecas): add a new getter to expose the import attributes as the value type
353+
// of [[RequestedModules]] is changed in https://tc39.es/proposal-import-attributes/#table-cyclic-module-fields.
354+
this[kDependencySpecifiers] ??= ArrayPrototypeMap(this[kWrap].getModuleRequests(), (request) => request.specifier);
334355
return this[kDependencySpecifiers];
335356
}
336357

@@ -392,10 +413,10 @@ class SyntheticModule extends Module {
392413
context,
393414
identifier,
394415
});
416+
}
395417

396-
this[kLink] = () => this[kWrap].link(() => {
397-
assert.fail('link callback should not be called');
398-
});
418+
[kLink]() {
419+
/** nothing to do for synthetic modules */
399420
}
400421

401422
setExport(name, value) {

‎src/env_properties.h

Copy file name to clipboardExpand all lines: src/env_properties.h
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
V(args_string, "args") \
7272
V(asn1curve_string, "asn1Curve") \
7373
V(async_ids_stack_string, "async_ids_stack") \
74+
V(attributes_string, "attributes") \
7475
V(base_string, "base") \
7576
V(bits_string, "bits") \
7677
V(block_list_string, "blockList") \
@@ -303,6 +304,7 @@
303304
V(sni_context_string, "sni_context") \
304305
V(source_string, "source") \
305306
V(source_map_url_string, "sourceMapURL") \
307+
V(specifier_string, "specifier") \
306308
V(stack_string, "stack") \
307309
V(standard_name_string, "standardName") \
308310
V(start_time_string, "startTime") \
@@ -377,6 +379,7 @@
377379
V(intervalhistogram_constructor_template, v8::FunctionTemplate) \
378380
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
379381
V(message_port_constructor_template, v8::FunctionTemplate) \
382+
V(module_wrap_constructor_template, v8::FunctionTemplate) \
380383
V(microtask_queue_ctor_template, v8::FunctionTemplate) \
381384
V(pipe_constructor_template, v8::FunctionTemplate) \
382385
V(promise_wrap_template, v8::ObjectTemplate) \

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.