revert: Revert "perf: improve bundle splitting" (#11468)

Revert "perf: improve bundle splitting (#11465)"

This reverts commit 65397a5.

Author: LingyuCoder

crates/rspack_plugin_split_chunks/src/module_group.rs

@@ -19,8 +19,9 @@ use crate::{
 /// The original name of `ModuleGroup` is `ChunkInfoItem` borrowed from Webpack
 #[derive(Debug)]
 pub(crate) struct ModuleGroup {
-  pub modules: IdentifierSet,
   /// the real index used for mapping the ModuleGroup to corresponding CacheGroup
+  idx: CacheGroupIdx,
+  pub modules: IdentifierSet,
   pub cache_group_index: usize,
   pub cache_group_priority: f64,
   pub cache_group_reuse_existing_chunk: bool,
@@ -36,11 +37,13 @@ pub(crate) struct ModuleGroup {
 
 impl ModuleGroup {
   pub fn new(
+    idx: CacheGroupIdx,
     chunk_name: Option<String>,
     cache_group_index: usize,
     cache_group: &CacheGroup,
   ) -> Self {
     Self {
+      idx,
       modules: Default::default(),
       cache_group_index,
       cache_group_priority: cache_group.priority,
@@ -113,14 +116,20 @@ impl ModuleGroup {
   }
 
   pub fn get_cache_group<'a>(&self, cache_groups: &'a [CacheGroup]) -> &'a CacheGroup {
-    &cache_groups[self.cache_group_index]
+    &cache_groups[self.idx.0]
   }
 }
 
-pub(crate) fn compare_entries(
-  (a_key, a): (&String, &ModuleGroup),
-  (b_key, b): (&String, &ModuleGroup),
-) -> f64 {
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub(crate) struct CacheGroupIdx(usize);
+
+impl CacheGroupIdx {
+  pub fn new(idx: usize) -> Self {
+    Self(idx)
+  }
+}
+
+pub(crate) fn compare_entries(a: &ModuleGroup, b: &ModuleGroup) -> f64 {
   // 1. by priority
   let diff_priority = a.cache_group_priority - b.cache_group_priority;
   if diff_priority != 0f64 {
@@ -161,7 +170,7 @@ pub(crate) fn compare_entries(
 
   loop {
     match (modules_a.pop(), modules_b.pop()) {
-      (None, None) => break,
+      (None, None) => return 0f64,
       (Some(a), Some(b)) => {
         let res = a.cmp(b);
         if !res.is_eq() {
@@ -171,8 +180,6 @@ pub(crate) fn compare_entries(
       _ => unreachable!(),
     }
   }
-
-  a_key.cmp(b_key) as i32 as f64
 }
 
 fn total_size(sizes: &SplitChunkSizes) -> f64 {

crates/rspack_plugin_split_chunks/src/plugin/module_group.rs

@@ -14,14 +14,14 @@ use rspack_core::{
 };
 use rspack_error::{Result, ToStringResultToRspackResultExt};
 use rspack_util::{fx_hash::FxDashMap, tracing_preset::TRACING_BENCH_TARGET};
-use rustc_hash::{FxHashMap, FxHasher};
+use rustc_hash::{FxHashMap, FxHashSet, FxHasher};
 use tracing::instrument;
 
 use super::ModuleGroupMap;
 use crate::{
   SplitChunksPlugin,
   common::{ModuleChunks, ModuleSizes},
-  module_group::{ModuleGroup, compare_entries},
+  module_group::{CacheGroupIdx, ModuleGroup, compare_entries},
   options::{
     cache_group::CacheGroup,
     cache_group_test::{CacheGroupTest, CacheGroupTestFnCtx},
@@ -34,6 +34,7 @@ type ChunksKey = u64;
 /// If a module meets requirements of a `ModuleGroup`. We consider the `Module` and the `CacheGroup`
 /// to be a `MatchedItem`, which are consumed later to calculate `ModuleGroup`.
 struct MatchedItem<'a> {
+  idx: CacheGroupIdx,
   module: &'a dyn Module,
   cache_group_index: usize,
   cache_group: &'a CacheGroup,
@@ -302,7 +303,7 @@ impl SplitChunksPlugin {
     let best_entry_key = module_group_map
       .iter()
       .min_by(|a, b| {
-        if compare_entries(*a, *b) < 0f64 {
+        if compare_entries(a.1, b.1) < 0f64 {
           Ordering::Greater
         } else {
           Ordering::Less
@@ -358,7 +359,9 @@ impl SplitChunksPlugin {
           let module = module_graph.module_by_identifier(module).expect("should have module").as_ref();
           let mut temp = Vec::with_capacity(plugin.cache_groups.len());
 
-          for cache_group in plugin.cache_groups.iter() {
+          for idx in 0..plugin.cache_groups.len() {
+            let cache_group = &plugin.cache_groups[idx];
+
             let mut is_match = true;
             // Filter by `splitChunks.cacheGroups.{cacheGroup}.type`
             is_match &= (cache_group.r#type)(module);
@@ -382,27 +385,17 @@ impl SplitChunksPlugin {
           }
           let mut chunk_key_to_string = HashMap::<ChunksKey, String, ChunksKeyHashBuilder>::default();
 
-          let mut filtered = plugin
+          let filtered = plugin
             .cache_groups
             .iter()
             .enumerate()
-            .filter(|(index, _)| temp[*index]).collect::<Vec<_>>();
-
-          filtered.sort_by(|a, b| {
-            b.1.priority.partial_cmp(&a.1.priority).unwrap_or_else(|| {
-              a.0.cmp(&b.0)
-            })
-          });
+            .filter(|(index, _)| temp[*index]);
 
           let mut used_exports_combs = None;
           let mut non_used_exports_combs = None;
-          let mut matched_max_priority = None;
-
-          for (cache_group_index, cache_group) in filtered.into_iter() {
-            if let Some(matched_max_priority) = matched_max_priority && cache_group.priority < matched_max_priority {
-              break;
-            }
+          let mut added_keys = FxHashSet::default();
 
+          for (cache_group_index, (idx, cache_group)) in filtered.enumerate() {
             let combs = if cache_group.used_exports {
               if used_exports_combs.is_none() {
                 used_exports_combs = Some(combinator.get_combs(
@@ -480,12 +473,9 @@ impl SplitChunksPlugin {
               let selected_chunks_key =
                 get_key(selected_chunks.iter().copied());
 
-              if matched_max_priority.is_none() {
-                matched_max_priority = Some(cache_group.priority);
-              }
-
               merge_matched_item_into_module_group_map(
                 MatchedItem {
+                  idx: CacheGroupIdx::new(idx),
                   module,
                   cache_group,
                   cache_group_index,
@@ -496,6 +486,7 @@ impl SplitChunksPlugin {
                 &mut chunk_key_to_string,
                 compilation,
                 module_sizes,
+                &mut added_keys,
               ).await?;
             }
           }
@@ -607,8 +598,10 @@ async fn merge_matched_item_into_module_group_map(
   chunk_key_to_string: &mut HashMap<ChunksKey, String, ChunksKeyHashBuilder>,
   compilation: &Compilation,
   module_sizes: &ModuleSizes,
+  added_keys: &mut FxHashSet<String>,
 ) -> Result<()> {
   let MatchedItem {
+    idx,
     module,
     cache_group_index,
     cache_group,
@@ -654,9 +647,11 @@ async fn merge_matched_item_into_module_group_map(
   let mut module_group = {
     module_group_map
       .entry(key.clone())
-      .or_insert_with(|| ModuleGroup::new(chunk_name, cache_group_index, cache_group))
+      .or_insert_with(|| ModuleGroup::new(idx, chunk_name.clone(), cache_group_index, cache_group))
   };
-  module_group.add_module(module.identifier(), module_sizes);
+  if chunk_name.is_none() || added_keys.insert(key) {
+    module_group.add_module(module.identifier(), module_sizes);
+  }
   module_group.chunks.extend(selected_chunks.iter().copied());
 
   Ok(())

tests/e2e/cases/css/chunk-loading-order/index.test.ts

@@ -2,5 +2,5 @@ import { test, expect } from "@/fixtures";
 
 test("should be red", async ({ page }) => {
 	await expect(page.locator("#status")).toHaveText("ok");
-	await expect(page.locator("body")).toHaveCSS("color", "rgb(0, 0, 255)");
+	await expect(page.locator("body")).toHaveCSS("color", "rgb(255, 0, 0)");
 });

tests/webpack-test/__snapshots__/StatsTestCases.basictest.js.snap

@@ -2521,22 +2521,20 @@ custom-chunks-filter:
 
 custom-chunks-filter-in-cache-groups:
   Entrypoint main xx KiB = custom-xxx.js
-  Entrypoint a xx KiB = custom-xxx.js
+  Entrypoint a xx KiB = custom-xxx.js xx KiB
   Entrypoint b xx KiB = custom-xxx.js xx KiB
   Entrypoint c xx KiB = custom-xxx.js xx KiB
-  chunk (runtime: a, main) custom-xxx.js (async-g) xx bytes <{341}> <{545}> <{724}> [rendered]
+  chunk (runtime: a, main) custom-xxx.js (async-g) xx bytes <{341}> <{545}> <{70}> <{724}> [rendered]
     > ./g ./a.js 6:0-47
     dependent modules xx bytes [dependent] 1 module
     ./g.js xx bytes [built] [code generated]
-  chunk (runtime: a) custom-xxx.js (a) xx bytes (javascript) xx KiB (runtime) >{138}< [entry] [rendered]
+  chunk (runtime: a) custom-xxx.js (a) xx bytes (javascript) xx KiB (runtime) ={70}= >{138}< [entry] [rendered]
     > ./a a
     > x a
     > y a
     > z a
-    dependent modules xx bytes [dependent] 3 modules
-    cacheable modules xx bytes
-      ./a.js + 1 modules xx bytes [code generated]
-      ./node_modules/z.js xx bytes [built] [code generated]
+    dependent modules xx bytes [dependent] 1 module
+    ./a.js + 1 modules xx bytes [code generated]
   chunk (runtime: b) custom-xxx.js (b) xx bytes (javascript) xx KiB (runtime) ={545}= [entry] [rendered]
     > ./b b
     > x b
@@ -2559,6 +2557,14 @@ custom-chunks-filter-in-cache-groups:
     ./node_modules/x.js xx bytes [built] [code generated]
     ./node_modules/y.js xx bytes [built] [code generated]
     ./node_modules/z.js xx bytes [built] [code generated]
+  chunk (runtime: a) custom-xxx.js (id hint: vendors) xx bytes ={341}= >{138}< [initial] [rendered] split chunk (cache group: defaultVendors)
+    > ./a a
+    > x a
+    > y a
+    > z a
+    ./node_modules/x.js xx bytes [built] [code generated]
+    ./node_modules/y.js xx bytes [built] [code generated]
+    ./node_modules/z.js xx bytes [built] [code generated]
   chunk (runtime: main) custom-xxx.js (async-b) xx bytes <{889}> ={545}= [rendered]
     > ./b ./index.js 2:0-47
     dependent modules xx bytes [dependent] 2 modules