Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(splitChunks): splitChunks maxSize not work as expected if size of some type is small #9285

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JSerFeng
Copy link
Contributor

@JSerFeng JSerFeng commented Feb 13, 2025

Summary

Align: https://github.com/webpack/webpack/blob/3919c844eca394d73ca930e4fc5506fb86e2b094/lib/util/deterministicGrouping.js#L326.

there is a chunk contains following modules (use 0 represents module)

[0 0 0 0 0 0 0]

User set maxSize, and total size of these modules is greater than maxSize, so we need to split it

From left, we find a position which can just fulfill the minSize

From right, we also find a position which can just fulfill the minSize

Case 1:

Perfact splitting

[0 0 0 0 0 0 0]
[0 0 0]
 left^
      [0 0 0 0]
       ^right

Split it into 2 chunks, left and right are all greater than minSize

Case 2:

[0 0 0 0 0 0 0]
[0 0 0]
 left^
          [0 0]
           ^right

Split it based on some special algorithm, still, the 2 chunks are all greater than minSize, so it's fine

Case 3:

[0 0 0 0 0 0 0]
[0 0 0 0 0]
     left^
        [0 0 0]
         ^right

Left and Right has overlaps, if split left, right part doesn't satisfy minSize, for current version before this pr, we just ignore this, and do not split this chunk at all.

But when size is smaller than minSize, there is a possibility that not all module type's size is smaller than minSize,

eg. the right 3 modules has only 1 module that contains css, the total css size is smaller than minSize but the total js size is larger than minSize.

In this case move the module that contains that css into a existing result chunk, and re-check the maxSize

Align this logic in this pr

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copy link

netlify bot commented Feb 13, 2025

Deploy Preview for rspack ready!

Name Link
🔨 Latest commit bf4c20b
🔍 Latest deploy log https://app.netlify.com/sites/rspack/deploys/67b448282df9510007f3fd0b
😎 Deploy Preview https://deploy-preview-9285--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added release: feature release: feature related release(mr only) team The issue/pr is created by the member of Rspack. labels Feb 13, 2025
@JSerFeng JSerFeng enabled auto-merge (squash) February 13, 2025 10:55
Copy link

codspeed-hq bot commented Feb 13, 2025

CodSpeed Performance Report

Merging #9285 will not alter performance

Comparing feat/align-webpack (bf4c20b) with main (73688e8)

🎉 Hooray! codspeed-rust just leveled up to 2.7.2!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 7 untouched benchmarks

@JSerFeng JSerFeng changed the title feat(splitChunks): align webpack splitChunks maxSize fix(splitChunks): splitChunks maxSize not work as expected if size of some type is small Feb 13, 2025
@github-actions github-actions bot added release: bug fix release: bug related release(mr only) and removed release: feature release: feature related release(mr only) labels Feb 13, 2025
@bigbossx
Copy link

LGTM~ nice job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants