Skip to content

Commit

Permalink
lib,src: iterate module requests of a module wrap in JS
Browse files Browse the repository at this point in the history
Avoid repetitively calling into JS callback from C++ in
`ModuleWrap::Link`. This removes the convoluted callback style of the
internal `ModuleWrap` link step.
  • Loading branch information
legendecas committed Mar 12, 2024
1 parent 63d04d4 commit b201582
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 162 deletions.
56 changes: 33 additions & 23 deletions lib/internal/modules/esm/module_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,30 +78,8 @@ class ModuleJob {
this.modulePromise = PromiseResolve(this.modulePromise);
}

// Wait for the ModuleWrap instance being linked with all dependencies.
const link = async () => {
this.module = await this.modulePromise;
assert(this.module instanceof ModuleWrap);

// Explicitly keeping track of dependency jobs is needed in order
// to flatten out the dependency graph below in `_instantiate()`,
// so that circular dependencies can't cause a deadlock by two of
// these `link` callbacks depending on each other.
const dependencyJobs = [];
const promises = this.module.link(async (specifier, attributes) => {
const job = await this.loader.getModuleJob(specifier, url, attributes);
ArrayPrototypePush(dependencyJobs, job);
return job.modulePromise;
});

if (promises !== undefined) {
await SafePromiseAllReturnVoid(promises);
}

return SafePromiseAllReturnArrayLike(dependencyJobs);
};
// Promise for the list of all dependencyJobs.
this.linked = link();
this.linked = this._link();
// This promise is awaited later anyway, so silence
// 'unhandled rejection' warnings.
PromisePrototypeThen(this.linked, undefined, noop);
Expand All @@ -111,6 +89,38 @@ class ModuleJob {
this.instantiated = undefined;
}

/**
* Iterates the module requests and links with the loader.
* @returns {Promise<ModuleJob[]>} Dependency module jobs.
*/
async _link() {
this.module = await this.modulePromise;
assert(this.module instanceof ModuleWrap);

// Explicitly keeping track of dependency jobs is needed in order
// to flatten out the dependency graph below in `_instantiate()`,
// so that circular dependencies can't cause a deadlock by two of
// these `link` callbacks depending on each other.
const dependencyJobs = [];
const promises = [];
// Iterate with index to avoid calling into userspace with `Symbol.iterator`.
const moduleRequestsLength = this.module.moduleRequests.length;
for (let idx = 0; idx < moduleRequestsLength; idx++) {
const { specifier, attributes } = this.module.moduleRequests[idx];
const job = await this.loader.getModuleJob(specifier, this.url, attributes);
ArrayPrototypePush(dependencyJobs, job);
ArrayPrototypePush(promises, job.modulePromise);
this.module.linkModule(
specifier,
job.modulePromise,
/** finished */ idx === moduleRequestsLength - 1,
);
}

await SafePromiseAllReturnVoid(promises);
return SafePromiseAllReturnArrayLike(dependencyJobs);
}

instantiate() {
if (this.instantiated === undefined) {
this.instantiated = this._instantiate();
Expand Down
62 changes: 39 additions & 23 deletions lib/internal/vm/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ const {
ArrayIsArray,
ArrayPrototypeForEach,
ArrayPrototypeIndexOf,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeSome,
ObjectDefineProperty,
ObjectFreeze,
ObjectGetPrototypeOf,
ObjectSetPrototypeOf,
PromiseResolve,
PromisePrototypeThen,
ReflectApply,
SafePromiseAllReturnVoid,
Symbol,
Expand Down Expand Up @@ -306,27 +310,40 @@ class SourceTextModule extends Module {
this[kLink] = async (linker) => {
this.#statusOverride = 'linking';

const promises = this[kWrap].link(async (identifier, attributes) => {
const module = await linker(identifier, this, { attributes, assert: attributes });
if (module[kWrap] === undefined) {
throw new ERR_VM_MODULE_NOT_MODULE();
}
if (module.context !== this.context) {
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
}
if (module.status === 'errored') {
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${identifier}' resolved to an errored module`, module.error);
}
if (module.status === 'unlinked') {
await module[kLink](linker);
}
return module[kWrap];
});
// Iterates the module requests and links with the linker.
const promises = [];
// Iterate with index to avoid calling into userspace with `Symbol.iterator`.
const moduleRequestsLength = this[kWrap].moduleRequests.length;
for (let idx = 0; idx < moduleRequestsLength; idx++) {
const { specifier, attributes } = this[kWrap].moduleRequests[idx];

const linkerResult = linker(specifier, this, {
attributes,
assert: attributes,
});
const promise = PromisePrototypeThen(
PromiseResolve(linkerResult), async (module) => {
if (module[kWrap] === undefined) {
throw new ERR_VM_MODULE_NOT_MODULE();
}
if (module.context !== this.context) {
throw new ERR_VM_MODULE_DIFFERENT_CONTEXT();
}
if (module.status === 'errored') {
throw new ERR_VM_MODULE_LINK_FAILURE(`request for '${specifier}' resolved to an errored module`, module.error);
}
if (module.status === 'unlinked') {
await module[kLink](linker);
}
return module[kWrap];
});

this[kWrap].linkModule(specifier, promise, /** finished */ idx === moduleRequestsLength - 1);
ArrayPrototypePush(promises, promise);
}

try {
if (promises !== undefined) {
await SafePromiseAllReturnVoid(promises);
}
await SafePromiseAllReturnVoid(promises);
} catch (e) {
this.#error = e;
throw e;
Expand All @@ -342,7 +359,8 @@ class SourceTextModule extends Module {
if (this[kWrap] === undefined) {
throw new ERR_VM_MODULE_NOT_MODULE();
}
this[kDependencySpecifiers] ??= ObjectFreeze(this[kWrap].getStaticDependencySpecifiers());
this[kDependencySpecifiers] ??= ObjectFreeze(
ArrayPrototypeMap(this[kWrap].moduleRequests, (request) => request.specifier));
return this[kDependencySpecifiers];
}

Expand Down Expand Up @@ -409,9 +427,7 @@ class SyntheticModule extends Module {
identifier,
});

this[kLink] = () => this[kWrap].link(() => {
assert.fail('link callback should not be called');
});
this[kLink] = () => this[kWrap].markLinked();
}

setExport(name, value) {
Expand Down
3 changes: 3 additions & 0 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
V(args_string, "args") \
V(asn1curve_string, "asn1Curve") \
V(async_ids_stack_string, "async_ids_stack") \
V(attributes_string, "attributes") \
V(base_string, "base") \
V(bits_string, "bits") \
V(block_list_string, "blockList") \
Expand Down Expand Up @@ -213,6 +214,7 @@
V(mgf1_hash_algorithm_string, "mgf1HashAlgorithm") \
V(minttl_string, "minttl") \
V(module_string, "module") \
V(module_requests_string, "moduleRequests") \
V(modulus_string, "modulus") \
V(modulus_length_string, "modulusLength") \
V(name_string, "name") \
Expand Down Expand Up @@ -300,6 +302,7 @@
V(sni_context_string, "sni_context") \
V(source_string, "source") \
V(source_map_url_string, "sourceMapURL") \
V(specifier_string, "specifier") \
V(stack_string, "stack") \
V(standard_name_string, "standardName") \
V(start_time_string, "startTime") \
Expand Down
Loading

0 comments on commit b201582

Please sign in to comment.