-
Notifications
You must be signed in to change notification settings - Fork 74
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
refactor(dev): 🔥 reduce ensure2 chunk map size #1707
base: master
Are you sure you want to change the base?
Conversation
Walkthrough该变更主要涉及对 Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis pull request refactors the chunk map size reduction in the development environment. The change replaces the Changes
|
@@ -32,8 +33,9 @@ pub fn module_ensure_map(context: &Arc<Context>) -> anyhow::Result<BTreeMap<Stri | |||
.iter() | |||
.for_each(|chunk| { | |||
let deps_chunks = cg | |||
.installable_descendants_chunk(&chunk.id) | |||
.sync_dependencies_chunk(&chunk.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that sync_dependencies_chunk
provides the expected chunk IDs. This change may affect the dependency resolution logic, so verify that it aligns with the intended production behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/plugins/central_ensure.rs (1)
36-38
: 优化: 减少 ensure2 chunk map 大小的改动这个改动通过以下方式优化了依赖收集:
- 使用
sync_dependencies_chunk
替代了返回几乎所有 chunks 的installable_descendants_chunk
- 通过
chain(once(&chunk.id))
确保当前 chunk 被包含在依赖列表中这样的改动更符合生产环境的行为,同时也能有效减少 chunk map 的大小。
建议添加单元测试来验证:
- chunk map 大小确实减少
- 动态导入的模块仍能正确加载
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1707 +/- ##
==========================================
- Coverage 55.28% 55.28% -0.01%
==========================================
Files 175 175
Lines 17699 17700 +1
==========================================
Hits 9785 9785
- Misses 7914 7915 +1 ☔ View full report in Codecov by Sentry. |
the former
installable_descendants_chunk
returns almost all the chunk in this build.sync_dependencies_chunk
is the best choice and aligning with production behaviorsSummary by CodeRabbit