Revert "fix: tapable run in wrong closure (#8376)"

This reverts commit 75f9573.
web-infra-dev · Nov 18, 2024 · 65a6aba · 65a6aba
1 parent 159c9b2
commit 65a6aba
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 288 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/node_binding/Cargo.toml b/crates/node_binding/Cargo.toml
@@ -32,9 +32,11 @@ rspack_plugin_javascript = { version = "0.1.0", path = "../rspack_plugin_javascr
 rspack_util              = { version = "0.1.0", path = "../rspack_util" }
 
 rspack_tracing = { version = "0.1.0", path = "../rspack_tracing" }
+tokio          = { workspace = true, features = ["rt", "rt-multi-thread", "tracing"] }
 
 async-trait = { workspace = true }
 cow-utils   = { workspace = true }
+once_cell   = { workspace = true }
 tracing     = { workspace = true }
 
 napi        = { workspace = true }

diff --git a/crates/node_binding/src/plugins/interceptor.rs b/crates/node_binding/src/plugins/interceptor.rs
@@ -1,4 +1,5 @@
 use std::{
+  borrow::Cow,
   hash::Hash,
   sync::{Arc, RwLock},
 };
@@ -134,22 +135,28 @@ impl<T: 'static + JsValuesTupleIntoVec, R> Clone for RegisterJsTapsInner<T, R> {
 
 enum RegisterJsTapsCache<T: 'static + JsValuesTupleIntoVec, R> {
   NoCache,
-  Cache(Arc<RwLock<Option<RegisterFunctionOutput<T, R>>>>),
+  Cache(Arc<tokio::sync::OnceCell<RegisterFunctionOutput<T, R>>>),
+  SyncCache(Arc<once_cell::sync::OnceCell<RegisterFunctionOutput<T, R>>>),
 }
 
 impl<T: 'static + JsValuesTupleIntoVec, R> Clone for RegisterJsTapsCache<T, R> {
   fn clone(&self) -> Self {
     match self {
       Self::NoCache => Self::NoCache,
       Self::Cache(c) => Self::Cache(c.clone()),
+      Self::SyncCache(c) => Self::SyncCache(c.clone()),
     }
   }
 }
 
 impl<T: 'static + JsValuesTupleIntoVec, R> RegisterJsTapsCache<T, R> {
-  pub fn new(cache: bool, _sync: bool) -> Self {
+  pub fn new(cache: bool, sync: bool) -> Self {
     if cache {
-      Self::Cache(Default::default())
+      if sync {
+        Self::SyncCache(Default::default())
+      } else {
+        Self::Cache(Default::default())
+      }
     } else {
       Self::NoCache
     }
@@ -173,27 +180,15 @@ impl<T: 'static + ToNapiValue, R: 'static + FromNapiValue> RegisterJsTapsInner<T
   pub async fn call_register(
     &self,
     hook: &impl Hook,
-  ) -> rspack_error::Result<RegisterFunctionOutput<T, R>> {
-    if let RegisterJsTapsCache::Cache(rw) = &self.cache {
-      let cache = {
-        #[allow(clippy::unwrap_used)]
-        rw.read().unwrap().clone()
-      };
-      Ok(match cache {
-        Some(js_taps) => js_taps,
-        None => {
-          let js_taps = self.call_register_impl(hook).await?;
-          {
-            #[allow(clippy::unwrap_used)]
-            let mut cache = rw.write().unwrap();
-            *cache = Some(js_taps.clone());
-          }
-          js_taps
-        }
-      })
+  ) -> rspack_error::Result<Cow<RegisterFunctionOutput<T, R>>> {
+    if let RegisterJsTapsCache::Cache(cache) = &self.cache {
+      let js_taps = cache
+        .get_or_try_init(|| self.call_register_impl(hook))
+        .await?;
+      Ok(Cow::Borrowed(js_taps))
     } else {
       let js_taps = self.call_register_impl(hook).await?;
-      Ok(js_taps)
+      Ok(Cow::Owned(js_taps))
     }
   }
 
@@ -209,27 +204,13 @@ impl<T: 'static + ToNapiValue, R: 'static + FromNapiValue> RegisterJsTapsInner<T
   pub fn call_register_blocking(
     &self,
     hook: &impl Hook,
-  ) -> rspack_error::Result<RegisterFunctionOutput<T, R>> {
-    if let RegisterJsTapsCache::Cache(rw) = &self.cache {
-      let cache = {
-        #[allow(clippy::unwrap_used)]
-        rw.read().unwrap().clone()
-      };
-      Ok(match cache {
-        Some(js_taps) => js_taps,
-        None => {
-          let js_taps = self.call_register_blocking_impl(hook)?;
-          {
-            #[allow(clippy::unwrap_used)]
-            let mut cache = rw.write().unwrap();
-            *cache = Some(js_taps.clone());
-          }
-          js_taps
-        }
-      })
+  ) -> rspack_error::Result<Cow<RegisterFunctionOutput<T, R>>> {
+    if let RegisterJsTapsCache::SyncCache(cache) = &self.cache {
+      let js_taps = cache.get_or_try_init(|| self.call_register_blocking_impl(hook))?;
+      Ok(Cow::Borrowed(js_taps))
     } else {
       let js_taps = self.call_register_blocking_impl(hook)?;
-      Ok(js_taps)
+      Ok(Cow::Owned(js_taps))
     }
   }
 
@@ -241,17 +222,6 @@ impl<T: 'static + ToNapiValue, R: 'static + FromNapiValue> RegisterJsTapsInner<T
     used_stages.sort();
     self.register.blocking_call_with_sync(used_stages)
   }
-
-  fn clear_cache(&self) {
-    match &self.cache {
-      RegisterJsTapsCache::NoCache => {}
-      RegisterJsTapsCache::Cache(cache) => {
-        #[allow(clippy::unwrap_used)]
-        let mut cache = cache.write().unwrap();
-        *cache = None;
-      }
-    }
-  }
 }
 
 /// define js taps register
@@ -273,12 +243,6 @@ macro_rules! define_register {
       inner: RegisterJsTapsInner<$arg, $ret>,
     }
 
-    impl $name {
-      pub fn clear_cache(&self) {
-        self.inner.clear_cache();
-      }
-    }
-
     #[derive(Clone)]
     struct $tap_name {
       function: ThreadsafeFunction<$arg, $ret>,

diff --git a/crates/node_binding/src/plugins/mod.rs b/crates/node_binding/src/plugins/mod.rs
@@ -312,90 +312,6 @@ impl rspack_core::Plugin for JsHooksAdapterPlugin {
 
     Ok(())
   }
-
-  fn clear_cache(&self) {
-    self.register_compiler_this_compilation_taps.clear_cache();
-    self.register_compiler_compilation_taps.clear_cache();
-    self.register_compiler_make_taps.clear_cache();
-    self.register_compiler_finish_make_taps.clear_cache();
-    self.register_compiler_should_emit_taps.clear_cache();
-    self.register_compiler_emit_taps.clear_cache();
-    self.register_compiler_after_emit_taps.clear_cache();
-    self.register_compiler_asset_emitted_taps.clear_cache();
-    self.register_compilation_build_module_taps.clear_cache();
-    self
-      .register_compilation_still_valid_module_taps
-      .clear_cache();
-    self.register_compilation_succeed_module_taps.clear_cache();
-    self.register_compilation_execute_module_taps.clear_cache();
-    self.register_compilation_finish_modules_taps.clear_cache();
-    self
-      .register_compilation_optimize_modules_taps
-      .clear_cache();
-    self
-      .register_compilation_after_optimize_modules_taps
-      .clear_cache();
-    self.register_compilation_optimize_tree_taps.clear_cache();
-    self
-      .register_compilation_optimize_chunk_modules_taps
-      .clear_cache();
-    self
-      .register_compilation_additional_tree_runtime_requirements
-      .clear_cache();
-    self
-      .register_compilation_runtime_requirement_in_tree
-      .clear_cache();
-    self.register_compilation_runtime_module_taps.clear_cache();
-    self.register_compilation_chunk_hash_taps.clear_cache();
-    self.register_compilation_chunk_asset_taps.clear_cache();
-    self.register_compilation_process_assets_taps.clear_cache();
-    self
-      .register_compilation_after_process_assets_taps
-      .clear_cache();
-    self.register_compilation_seal_taps.clear_cache();
-    self.register_compilation_after_seal_taps.clear_cache();
-    self
-      .register_normal_module_factory_before_resolve_taps
-      .clear_cache();
-    self
-      .register_normal_module_factory_factorize_taps
-      .clear_cache();
-    self
-      .register_normal_module_factory_resolve_taps
-      .clear_cache();
-    self
-      .register_normal_module_factory_resolve_for_scheme_taps
-      .clear_cache();
-    self
-      .register_normal_module_factory_after_resolve_taps
-      .clear_cache();
-    self
-      .register_normal_module_factory_create_module_taps
-      .clear_cache();
-    self
-      .register_context_module_factory_before_resolve_taps
-      .clear_cache();
-    self
-      .register_context_module_factory_after_resolve_taps
-      .clear_cache();
-    self
-      .register_javascript_modules_chunk_hash_taps
-      .clear_cache();
-    self
-      .register_html_plugin_before_asset_tag_generation_taps
-      .clear_cache();
-    self
-      .register_html_plugin_alter_asset_tags_taps
-      .clear_cache();
-    self
-      .register_html_plugin_alter_asset_tag_groups_taps
-      .clear_cache();
-    self
-      .register_html_plugin_after_template_execution_taps
-      .clear_cache();
-    self.register_html_plugin_before_emit_taps.clear_cache();
-    self.register_html_plugin_after_emit_taps.clear_cache();
-  }
 }
 
 #[plugin_hook(CompilerCompilation for JsHooksAdapterPlugin)]

diff --git a/crates/rspack_core/src/compiler/hmr.rs b/crates/rspack_core/src/compiler/hmr.rs
@@ -61,8 +61,7 @@ impl Compiler {
       self
         .old_cache
         .set_modified_files(all_files.into_iter().collect());
-
-      self.plugin_driver.clear_cache();
+      self.plugin_driver.resolver_factory.clear_cache();
 
       let mut new_compilation = Compilation::new(
         self.options.clone(),

diff --git a/crates/rspack_core/src/compiler/mod.rs b/crates/rspack_core/src/compiler/mod.rs
@@ -143,7 +143,7 @@ impl Compiler {
     self.old_cache.end_idle();
     // TODO: clear the outdated cache entries in resolver,
     // TODO: maybe it's better to use external entries.
-    self.plugin_driver.clear_cache();
+    self.plugin_driver.resolver_factory.clear_cache();
 
     let module_executor = ModuleExecutor::default();
     fast_set(

diff --git a/crates/rspack_core/src/plugin/mod.rs b/crates/rspack_core/src/plugin/mod.rs
@@ -22,8 +22,6 @@ pub trait Plugin: fmt::Debug + Send + Sync {
   ) -> Result<()> {
     Ok(())
   }
-
-  fn clear_cache(&self) {}
 }
 
 pub type BoxPlugin = Box<dyn Plugin>;

diff --git a/crates/rspack_core/src/plugin/plugin_driver.rs b/crates/rspack_core/src/plugin/plugin_driver.rs
@@ -76,11 +76,4 @@ impl PluginDriver {
     let mut diagnostic = self.diagnostics.lock().expect("TODO:");
     std::mem::take(&mut diagnostic)
   }
-
-  pub fn clear_cache(&self) {
-    self.resolver_factory.clear_cache();
-    for plugin in &self.plugins {
-      plugin.clear_cache();
-    }
-  }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -22,8 +22,6 @@ pub trait Plugin: fmt::Debug + Send + Sync { @@
       ) -> Result<()> {
         Ok(())
       }
-      fn clear_cache(&self) {}
     }
     pub type BoxPlugin = Box<dyn Plugin>;
@@ Expand Down @@