fix(core/modules): Prepare modules only once per runtime (#11015)

This commit changes module loading implementation in "deno_core"
to call "ModuleLoader::prepare" hook only once per entry point.

This is done to avoid multiple type checking of the same code
in case of duplicated dynamic imports.

Relevant code in "cli/module_graph.rs" was updated as well.

1 files changed, 93 insertions, 96 deletions

diff --git a/core/modules.rs b/core/modules.rs
index 642dcc26a..d1da67e89 100644
--- a/core/modules.rs
+++ b/core/modules.rs
@@ -180,16 +180,21 @@ impl ModuleLoader for FsModuleLoader {
   }
 }
 
-#[derive(Debug, Eq, PartialEq)]
-enum Kind {
-  Main,
-  DynamicImport,
+/// Describes the entrypoint of a recursive module load.
+#[derive(Debug)]
+enum LoadInit {
+  /// Main module specifier.
+  Main(String),
+  /// Main module specifier with synthetic code for that module which bypasses
+  /// the loader.
+  MainWithCode(String, String),
+  /// Dynamic import specifier with referrer.
+  DynamicImport(String, String),
 }
 
 #[derive(Debug, Eq, PartialEq)]
 pub enum LoadState {
-  ResolveMain(String, Option<String>),
-  ResolveImport(String, String),
+  Init,
   LoadingRoot,
   LoadingImports,
   Done,
@@ -198,7 +203,7 @@ pub enum LoadState {
 /// This future is used to implement parallel async module loading.
 pub struct RecursiveModuleLoad {
   op_state: Rc<RefCell<OpState>>,
-  kind: Kind,
+  init: LoadInit,
   // TODO(bartlomieju): in future this value should
   // be randomized
   pub id: ModuleLoadId,
@@ -217,9 +222,12 @@ impl RecursiveModuleLoad {
     code: Option<String>,
     loader: Rc<dyn ModuleLoader>,
   ) -> Self {
-    let kind = Kind::Main;
-    let state = LoadState::ResolveMain(specifier.to_owned(), code);
-    Self::new(op_state, kind, state, loader)
+    let init = if let Some(code) = code {
+      LoadInit::MainWithCode(specifier.to_string(), code)
+    } else {
+      LoadInit::Main(specifier.to_string())
+    };
+    Self::new(op_state, init, loader)
   }
 
   pub fn dynamic_import(
@@ -228,63 +236,54 @@ impl RecursiveModuleLoad {
     referrer: &str,
     loader: Rc<dyn ModuleLoader>,
   ) -> Self {
-    let kind = Kind::DynamicImport;
-    let state =
-      LoadState::ResolveImport(specifier.to_owned(), referrer.to_owned());
-    Self::new(op_state, kind, state, loader)
+    let init =
+      LoadInit::DynamicImport(specifier.to_string(), referrer.to_string());
+    Self::new(op_state, init, loader)
   }
 
   pub fn is_dynamic_import(&self) -> bool {
-    self.kind != Kind::Main
+    matches!(self.init, LoadInit::DynamicImport(..))
   }
 
   fn new(
     op_state: Rc<RefCell<OpState>>,
-    kind: Kind,
-    state: LoadState,
+    init: LoadInit,
     loader: Rc<dyn ModuleLoader>,
   ) -> Self {
     Self {
       id: NEXT_LOAD_ID.fetch_add(1, Ordering::SeqCst),
       root_module_id: None,
       op_state,
-      kind,
-      state,
+      init,
+      state: LoadState::Init,
       loader,
       pending: FuturesUnordered::new(),
       is_pending: HashSet::new(),
     }
   }
 
-  pub async fn prepare(self) -> (ModuleLoadId, Result<Self, AnyError>) {
-    let (module_specifier, maybe_referrer) = match self.state {
-      LoadState::ResolveMain(ref specifier, _) => {
+  pub async fn prepare(&self) -> Result<(), AnyError> {
+    let (module_specifier, maybe_referrer) = match self.init {
+      LoadInit::Main(ref specifier)
+      | LoadInit::MainWithCode(ref specifier, _) => {
         let spec =
-          match self
+          self
             .loader
-            .resolve(self.op_state.clone(), specifier, ".", true)
-          {
-            Ok(spec) => spec,
-            Err(e) => return (self.id, Err(e)),
-          };
+            .resolve(self.op_state.clone(), specifier, ".", true)?;
         (spec, None)
       }
-      LoadState::ResolveImport(ref specifier, ref referrer) => {
-        let spec = match self.loader.resolve(
+      LoadInit::DynamicImport(ref specifier, ref referrer) => {
+        let spec = self.loader.resolve(
           self.op_state.clone(),
           specifier,
           referrer,
           false,
-        ) {
-          Ok(spec) => spec,
-          Err(e) => return (self.id, Err(e)),
-        };
+        )?;
         (spec, Some(referrer.to_string()))
       }
-      _ => unreachable!(),
     };
 
-    let prepare_result = self
+    self
       .loader
       .prepare_load(
         self.op_state.clone(),
@@ -293,52 +292,7 @@ impl RecursiveModuleLoad {
         maybe_referrer,
         self.is_dynamic_import(),
       )
-      .await;
-
-    match prepare_result {
-      Ok(()) => (self.id, Ok(self)),
-      Err(e) => (self.id, Err(e)),
-    }
-  }
-
-  fn add_root(&mut self) -> Result<(), AnyError> {
-    let module_specifier = match self.state {
-      LoadState::ResolveMain(ref specifier, _) => {
-        self
-          .loader
-          .resolve(self.op_state.clone(), specifier, ".", true)?
-      }
-      LoadState::ResolveImport(ref specifier, ref referrer) => self
-        .loader
-        .resolve(self.op_state.clone(), specifier, referrer, false)?,
-
-      _ => unreachable!(),
-    };
-
-    let load_fut = match &self.state {
-      LoadState::ResolveMain(_, Some(code)) => {
-        futures::future::ok(ModuleSource {
-          code: code.to_owned(),
-          module_url_specified: module_specifier.to_string(),
-          module_url_found: module_specifier.to_string(),
-        })
-        .boxed()
-      }
-      _ => self
-        .loader
-        .load(
-          self.op_state.clone(),
-          &module_specifier,
-          None,
-          self.is_dynamic_import(),
-        )
-        .boxed_local(),
-    };
-
-    self.pending.push(load_fut);
-
-    self.state = LoadState::LoadingRoot;
-    Ok(())
+      .await
   }
 
   pub fn is_currently_loading_main_module(&self) -> bool {
@@ -390,10 +344,38 @@ impl Stream for RecursiveModuleLoad {
   ) -> Poll<Option<Self::Item>> {
     let inner = self.get_mut();
     match inner.state {
-      LoadState::ResolveMain(..) | LoadState::ResolveImport(..) => {
-        if let Err(e) = inner.add_root() {
-          return Poll::Ready(Some(Err(e)));
-        }
+      LoadState::Init => {
+        let resolve_result = match inner.init {
+          LoadInit::Main(ref specifier)
+          | LoadInit::MainWithCode(ref specifier, _) => {
+            inner
+              .loader
+              .resolve(inner.op_state.clone(), specifier, ".", true)
+          }
+          LoadInit::DynamicImport(ref specifier, ref referrer) => inner
+            .loader
+            .resolve(inner.op_state.clone(), specifier, referrer, false),
+        };
+        let module_specifier = match resolve_result {
+          Ok(url) => url,
+          Err(error) => return Poll::Ready(Some(Err(error))),
+        };
+        let load_fut = match inner.init {
+          LoadInit::MainWithCode(_, ref code) => {
+            futures::future::ok(ModuleSource {
+              code: code.clone(),
+              module_url_specified: module_specifier.to_string(),
+              module_url_found: module_specifier.to_string(),
+            })
+            .boxed()
+          }
+          LoadInit::Main(..) | LoadInit::DynamicImport(..) => inner
+            .loader
+            .load(inner.op_state.clone(), &module_specifier, None, false)
+            .boxed_local(),
+        };
+        inner.pending.push(load_fut);
+        inner.state = LoadState::LoadingRoot;
         inner.try_poll_next_unpin(cx)
       }
       LoadState::LoadingRoot | LoadState::LoadingImports => {
@@ -648,17 +630,19 @@ impl ModuleMap {
     self.info.get(id)
   }
 
-  pub fn load_main(
+  pub async fn load_main(
     &self,
     specifier: &str,
     code: Option<String>,
-  ) -> RecursiveModuleLoad {
-    RecursiveModuleLoad::main(
+  ) -> Result<RecursiveModuleLoad, AnyError> {
+    let load = RecursiveModuleLoad::main(
       self.op_state.clone(),
       specifier,
       code,
       self.loader.clone(),
-    )
+    );
+    load.prepare().await?;
+    Ok(load)
   }
 
   // Initiate loading of a module graph imported using `import()`.
@@ -675,7 +659,21 @@ impl ModuleMap {
       self.loader.clone(),
     );
     self.dynamic_import_map.insert(load.id, resolver_handle);
-    let fut = load.prepare().boxed_local();
+    let resolve_result =
+      load
+        .loader
+        .resolve(load.op_state.clone(), specifier, referrer, false);
+    let fut = match resolve_result {
+      Ok(module_specifier) => {
+        if self.is_registered(&module_specifier) {
+          async move { (load.id, Ok(load)) }.boxed_local()
+        } else {
+          async move { (load.id, load.prepare().await.map(|()| load)) }
+            .boxed_local()
+        }
+      }
+      Err(error) => async move { (load.id, Err(error)) }.boxed_local(),
+    };
     self.preparing_dynamic_imports.push(fut);
   }
 
@@ -1128,13 +1126,12 @@ mod tests {
         )
         .unwrap();
 
-      assert_eq!(count.load(Ordering::Relaxed), 0);
       // We should get an error here.
       let result = runtime.poll_event_loop(cx, false);
       if let Poll::Ready(Ok(_)) = result {
         unreachable!();
       }
-      assert_eq!(count.load(Ordering::Relaxed), 2);
+      assert_eq!(count.load(Ordering::Relaxed), 3);
     })
   }
 
@@ -1154,7 +1151,7 @@ mod tests {
       _is_main: bool,
     ) -> Result<ModuleSpecifier, AnyError> {
       let c = self.resolve_count.fetch_add(1, Ordering::Relaxed);
-      assert!(c < 4);
+      assert!(c < 5);
       assert_eq!(specifier, "./b.js");
       assert_eq!(referrer, "file:///dyn_import3.js");
       let s = crate::resolve_import(specifier, referrer).unwrap();
@@ -1231,13 +1228,13 @@ mod tests {
         runtime.poll_event_loop(cx, false),
         Poll::Ready(Ok(_))
       ));
-      assert_eq!(resolve_count.load(Ordering::Relaxed), 4);
+      assert_eq!(resolve_count.load(Ordering::Relaxed), 5);
       assert_eq!(load_count.load(Ordering::Relaxed), 2);
       assert!(matches!(
         runtime.poll_event_loop(cx, false),
         Poll::Ready(Ok(_))
       ));
-      assert_eq!(resolve_count.load(Ordering::Relaxed), 4);
+      assert_eq!(resolve_count.load(Ordering::Relaxed), 5);
       assert_eq!(load_count.load(Ordering::Relaxed), 2);
     })
   }