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.

Author: legendecas

lib/internal/modules/esm/module_job.js

@@ -1,8 +1,8 @@
 'use strict';
 
 const {
+  Array,
   ArrayPrototypeJoin,
-  ArrayPrototypePush,
   ArrayPrototypeSome,
   FunctionPrototype,
   ObjectSetPrototypeOf,
@@ -83,30 +83,8 @@ class ModuleJob extends ModuleJobBase {
       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);
@@ -116,6 +94,48 @@ class ModuleJob extends ModuleJobBase {
     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);
+
+    const moduleRequestsLength = this.module.moduleRequests.length;
+    // 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.
+    // Create an ArrayLike to avoid calling into userspace with `.then`
+    // when returned from the async function.
+    const dependencyJobs = Array(moduleRequestsLength);
+    ObjectSetPrototypeOf(dependencyJobs, null);
+
+    // Specifiers should be aligned with the moduleRequests array in order.
+    const specifiers = Array(moduleRequestsLength);
+    const modulePromises = Array(moduleRequestsLength);
+    // Iterate with index to avoid calling into userspace with `Symbol.iterator`.
+    for (let idx = 0; idx < moduleRequestsLength; idx++) {
+      const { specifier, attributes } = this.module.moduleRequests[idx];
+
+      const dependencyJobPromise = this.loader.getModuleJob(
+        specifier, this.url, attributes,
+      );
+      const modulePromise = PromisePrototypeThen(dependencyJobPromise, (job) => {
+        dependencyJobs[idx] = job;
+        return job.modulePromise;
+      });
+      modulePromises[idx] = modulePromise;
+      specifiers[idx] = specifier;
+    }
+
+    const modules = await SafePromiseAllReturnArrayLike(modulePromises);
+    this.module.link(specifiers, modules);
+
+    return dependencyJobs;
+  }
+
   instantiate() {
     if (this.instantiated === undefined) {
       this.instantiated = this._instantiate();
@@ -268,15 +288,17 @@ class ModuleJobSync extends ModuleJobBase {
   constructor(loader, url, importAttributes, moduleWrap, isMain, inspectBrk) {
     super(loader, url, importAttributes, moduleWrap, isMain, inspectBrk, true);
     assert(this.module instanceof ModuleWrap);
-    const moduleRequests = this.module.getModuleRequestsSync();
-    for (let i = 0; i < moduleRequests.length; ++i) {
-      const { 0: specifier, 1: attributes } = moduleRequests[i];
+    const moduleRequestsLength = this.module.moduleRequests.length;
+    // Specifiers should be aligned with the moduleRequests array in order.
+    const specifiers = Array(moduleRequestsLength);
+    const modules = Array(moduleRequestsLength);
+    for (let i = 0; i < moduleRequestsLength; ++i) {
+      const { specifier, attributes } = this.module.moduleRequests[i];
       const wrap = this.loader.getModuleWrapForRequire(specifier, url, attributes);
-      const isLast = (i === moduleRequests.length - 1);
-      // TODO(joyeecheung): make the resolution callback deal with both promisified
-      // an raw module wraps, then we don't need to wrap it with a promise here.
-      this.module.cacheResolvedWrapsSync(specifier, PromiseResolve(wrap), isLast);
+      specifiers[i] = specifier;
+      modules[i] = wrap;
     }
+    this.module.link(specifiers, modules);
   }
 
   async run() {

lib/internal/vm/module.js

@@ -2,16 +2,20 @@
 
 const assert = require('internal/assert');
 const {
+  Array,
   ArrayIsArray,
   ArrayPrototypeForEach,
   ArrayPrototypeIndexOf,
+  ArrayPrototypeMap,
   ArrayPrototypeSome,
   ObjectDefineProperty,
   ObjectFreeze,
   ObjectGetPrototypeOf,
   ObjectSetPrototypeOf,
+  PromiseResolve,
+  PromisePrototypeThen,
   ReflectApply,
-  SafePromiseAllReturnVoid,
+  SafePromiseAllReturnArrayLike,
   Symbol,
   SymbolToStringTag,
   TypeError,
@@ -303,46 +307,62 @@ class SourceTextModule extends Module {
       importModuleDynamically,
     });
 
-    this[kLink] = async (linker) => {
-      this.#statusOverride = 'linking';
+    this[kDependencySpecifiers] = undefined;
+  }
 
-      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];
+  async [kLink](linker) {
+    this.#statusOverride = 'linking';
+
+    const moduleRequestsLength = this[kWrap].moduleRequests.length;
+    // Iterates the module requests and links with the linker.
+    // Specifiers should be aligned with the moduleRequests array in order.
+    const specifiers = Array(moduleRequestsLength);
+    const modulePromises = Array(moduleRequestsLength);
+    // Iterates with index to avoid calling into userspace with `Symbol.iterator`.
+    for (let idx = 0; idx < moduleRequestsLength; idx++) {
+      const { specifier, attributes } = this[kWrap].moduleRequests[idx];
+
+      const linkerResult = linker(specifier, this, {
+        attributes,
+        assert: attributes,
       });
-
-      try {
-        if (promises !== undefined) {
-          await SafePromiseAllReturnVoid(promises);
-        }
-      } catch (e) {
-        this.#error = e;
-        throw e;
-      } finally {
-        this.#statusOverride = undefined;
-      }
-    };
-
-    this[kDependencySpecifiers] = undefined;
+      const modulePromise = 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];
+        });
+      modulePromises[idx] = modulePromise;
+      specifiers[idx] = specifier;
+    }
+
+    try {
+      const modules = await SafePromiseAllReturnArrayLike(modulePromises);
+      this[kWrap].link(specifiers, modules);
+    } catch (e) {
+      this.#error = e;
+      throw e;
+    } finally {
+      this.#statusOverride = undefined;
+    }
   }
 
   get dependencySpecifiers() {
     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];
   }
 
@@ -408,10 +428,10 @@ class SyntheticModule extends Module {
       context,
       identifier,
     });
+  }
 
-    this[kLink] = () => this[kWrap].link(() => {
-      assert.fail('link callback should not be called');
-    });
+  [kLink]() {
+    /** nothing to do for synthetic modules */
   }
 
   setExport(name, value) {

src/env_properties.h

@@ -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")                                            \
@@ -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")                                                       \
@@ -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")                                            \
@@ -377,6 +380,7 @@
   V(js_transferable_constructor_template, v8::FunctionTemplate)                \
   V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate)                     \
   V(message_port_constructor_template, v8::FunctionTemplate)                   \
+  V(module_wrap_constructor_template, v8::FunctionTemplate)                    \
   V(microtask_queue_ctor_template, v8::FunctionTemplate)                       \
   V(pipe_constructor_template, v8::FunctionTemplate)                           \
   V(promise_wrap_template, v8::ObjectTemplate)                                 \