FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib,src: iterate module requests of a module wrap in JS · nodejs/node@f13589f · GitHub

/ node Public

Commit f13589f

Browse files
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

9 files changed

Lines changed: 219 additions & 229 deletions

File tree

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

Lines 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‎

Lines 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‎

Lines 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‎

Lines 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)
Back | FazBrowse Home | New Git URL