From 418d386cf5f0324c524992a5a3666509f5964df6 Mon Sep 17 00:00:00 2001 From: jserfeng <1114550440@qq.com> Date: Fri, 14 Feb 2025 16:21:35 +0800 Subject: [PATCH] feat: sort items to keep deterministic --- .../src/plugin/max_size.rs | 78 +++++++++++-------- .../max-size-split-with-css/rspack.config.js | 35 ++++++--- .../max-size-split-with-css/src/aaa/index.js | 1 + .../max-size-split-with-css/src/aaa/small.css | 10 +++ .../max-size-split-with-css/src/small.css | 10 +++ .../max-size-split-with-css/test.config.js | 13 +++- 6 files changed, 100 insertions(+), 47 deletions(-) create mode 100644 packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/small.css diff --git a/crates/rspack_plugin_split_chunks/src/plugin/max_size.rs b/crates/rspack_plugin_split_chunks/src/plugin/max_size.rs index 4d34cd1b013c..c95a31028941 100644 --- a/crates/rspack_plugin_split_chunks/src/plugin/max_size.rs +++ b/crates/rspack_plugin_split_chunks/src/plugin/max_size.rs @@ -116,8 +116,6 @@ fn hash_filename(filename: &str, options: &CompilerOptions) -> String { hash_digest.rendered(8).to_string() } -static REPLACE_MODULE_IDENTIFIER_REG: LazyLock = - LazyLock::new(|| Regex::new(r"^.*!|\?[^?!]*$").expect("regexp init failed")); static REPLACE_RELATIVE_PREFIX_REG: LazyLock = LazyLock::new(|| Regex::new(r"^(\.\.?\/)+").expect("regexp init failed")); static REPLACE_ILLEGEL_LETTER_REG: LazyLock = @@ -144,7 +142,7 @@ fn get_too_small_types( if let Some(min_ty_size) = min_size.get(ty) && ty_size < min_ty_size { - types.insert(ty.clone()); + types.insert(*ty); } }); types @@ -156,7 +154,7 @@ fn remove_problematic_nodes( min_size: &SplitChunkSizes, result: &mut Vec, ) -> bool { - let problem_types = get_too_small_types(&considered_size, min_size); + let problem_types = get_too_small_types(considered_size, min_size); if !problem_types.is_empty() { // We hit an edge case where the working set is already smaller than minSize @@ -198,17 +196,18 @@ fn remove_problematic_nodes( .filter(|(ty, _)| problem_types.contains(ty)) .count(); - if min_matched < group_matched { - return group; - } else if min_matched > group_matched { - return min; - }; - - if sum_for_types(&min.size, &problem_types) > sum_for_types(&group.size, &problem_types) { - return group; + match min_matched.cmp(&group_matched) { + std::cmp::Ordering::Less => group, + std::cmp::Ordering::Greater => min, + std::cmp::Ordering::Equal => { + if sum_for_types(&min.size, &problem_types) > sum_for_types(&group.size, &problem_types) + { + group + } else { + min + } + } } - - min }); let best_group: &mut Group = @@ -231,6 +230,32 @@ fn sum_for_types(size: &SplitChunkSizes, ty: &FxHashSet) -> f64 { .sum() } +fn get_key(module: &dyn Module, delemeter: &str, compilation: &Compilation) -> String { + let ident = make_paths_relative( + compilation.options.context.as_str(), + module.identifier().as_str(), + ); + let name = if let Some(name_for_condition) = module.name_for_condition() { + Cow::Owned(make_paths_relative( + compilation.options.context.as_str(), + &name_for_condition, + )) + } else { + static RE: LazyLock = + LazyLock::new(|| Regex::new(r"^.*!|\?[^?!]*$").expect("should build regex")); + RE.replace_all(&ident, "") + }; + + let full_key = format!( + "{}{}{}", + name, + delemeter, + hash_filename(&ident, &compilation.options) + ); + + request_to_id(&full_key) +} + fn deterministic_grouping_for_modules( compilation: &Compilation, chunk: &ChunkUkey, @@ -244,32 +269,18 @@ fn deterministic_grouping_for_modules( let items = compilation .chunk_graph .get_chunk_modules(chunk, &module_graph); - let context = compilation.options.context.as_ref(); let nodes = items.into_iter().map(|module| { let module: &dyn Module = &**module; - let name: String = if let Some(name_for_condition) = module.name_for_condition() { - make_paths_relative(context, &name_for_condition) - } else { - let path = make_paths_relative(context, module.identifier().as_str()); - REPLACE_MODULE_IDENTIFIER_REG - .replace_all(&path, "") - .to_string() - }; - let key = format!( - "{}{}{}", - name, - delimiter, - hash_filename(&name, &compilation.options) - ); + GroupItem { module: module.identifier(), size: get_size(module, compilation), - key: request_to_id(&key), + key: get_key(module, delimiter, compilation), } }); - let initial_nodes = nodes + let mut initial_nodes = nodes .into_iter() .filter_map(|node| { // The Module itself is already bigger than `allow_max_size`, we will create a chunk @@ -291,6 +302,7 @@ fn deterministic_grouping_for_modules( .collect::>(); if !initial_nodes.is_empty() { + initial_nodes.sort_by(|a, b| a.key.cmp(&b.key)); let similarities = get_similarities(&initial_nodes); let initial_group = Group::new(initial_nodes, None, similarities); @@ -377,7 +389,7 @@ fn deterministic_grouping_for_modules( && right_size.bigger_than(min_size) { best_similarity = similarity; - best = pos as i32; + best = pos; } let size = &group.nodes[pos as usize].size; left_size.add_by(size); @@ -420,7 +432,7 @@ fn deterministic_grouping_for_modules( fn subtract_size_from(total: &mut SplitChunkSizes, size: &SplitChunkSizes) { size.iter().for_each(|(ty, ty_size)| { let total_ty_size = total.get(ty).copied().unwrap_or(0.0); - total.insert(ty.clone(), total_ty_size - ty_size); + total.insert(*ty, total_ty_size - ty_size); }); } diff --git a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/rspack.config.js b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/rspack.config.js index cd294d3957e1..a0b520d1755d 100644 --- a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/rspack.config.js +++ b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/rspack.config.js @@ -1,34 +1,49 @@ +const rspack = require("@rspack/core"); + /** @type {import("@rspack/core").Configuration} */ module.exports = { - target: "node", + target: "web", entry: "./src/index.js", output: { filename: "[name].js" }, experiments: { - css: true + css: false }, module: { - generator: { - "css/auto": { - exportsOnly: false + rules: [ + { + test: /\.css$/, + use: [rspack.CssExtractRspackPlugin.loader, "css-loader"] } - } + ] }, + plugins: [new rspack.CssExtractRspackPlugin()], + performance: false, optimization: { chunkIds: "named", + usedExports: false, sideEffects: false, + concatenateModules: false, moduleIds: "named", splitChunks: { chunks: "all", cacheGroups: { + default: false, + defaultVendors: false, fragment: { minChunks: 1, maxSize: 200 * 1024, - // user specifies that css size in chunk should also satisfy minSize - // which is not in this test, so the css module should be splitted - // and re-check the js part - minSize: 100, + + // there are 2 css, each one of them are only 120 bytes which is less than minSize + // so the total size of the css are 240 bytes which is greater than minSize + // so the nodes are + // [js js css js js js js js css] + // if scan from left to right, the minSize can only satisfy when scan to the last css + // if scan from right to left, the minSize can only satisfy when scan to the first css + // so split chunks should remove problematic nodes, in this case the 2 css + // and then recalculate the size of the rest of the nodes + minSize: 200, priority: 10 } } diff --git a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/index.js b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/index.js index 9968cd39962d..70a0e73f3c82 100644 --- a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/index.js +++ b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/index.js @@ -2,3 +2,4 @@ import './50k-1' import './50k-2' import './50k-3' import './50k-4' +import './small.css' diff --git a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/small.css b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/small.css new file mode 100644 index 000000000000..0f19baf023de --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/aaa/small.css @@ -0,0 +1,10 @@ +body { + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + color: red +} \ No newline at end of file diff --git a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/small.css b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/small.css index e69de29bb2d1..0f19baf023de 100644 --- a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/small.css +++ b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/src/small.css @@ -0,0 +1,10 @@ +body { + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + /* small.css */ + color: red +} \ No newline at end of file diff --git a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/test.config.js b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/test.config.js index 172d8ffa640b..6b03a5a69cf8 100644 --- a/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/test.config.js +++ b/packages/rspack-test-tools/tests/configCases/split-chunks/max-size-split-with-css/test.config.js @@ -3,10 +3,15 @@ module.exports = { findBundle: function (i, options) { // should split based on their file path return [ - "main.js", - "fragment-src_aaa_sync_recursive_.js", - "fragment-src_bbb_sync_recursive_.js", - "fragment-src_index_js.js" + // the total css size are not satisfied by minSize, so the css modules + // are split and try again to see if the reset size satisfied the minSize + // then its okay, so the js can be split + "fragment-src_aaa_index_js.js", + "fragment-src_aaa_small_css-src_small_css.css", + "fragment-src_bbb_index_js-src_aaa_small_css.js", + "fragment-src_ccc_50k-1_js-src_ccc_50k-2_js-src_ccc_50k-3_js-src_ccc_50k-4_js.js", + "fragment-src_index_js.js", + "main.js" ]; } };