-
Notifications
You must be signed in to change notification settings - Fork 73
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
perf: optimize group_chunks, speed up group_chunks by 500% #1475
Conversation
Walkthrough此次更改引入了新的依赖项 Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
Files selected for processing (4)
- crates/mako/Cargo.toml (1 hunks)
- crates/mako/src/generate/group_chunk.rs (2 hunks)
- crates/mako/src/module_graph/link_back_dfs_visit.rs (1 hunks)
- crates/mako/src/module_graph/mod.rs (2 hunks)
Additional comments not posted (24)
crates/mako/src/module_graph/link_back_dfs_visit.rs (6)
13-18
: 结构定义良好
LinkBackDfs
结构体定义正确,字段类型适用于预期用途。
20-25
: 方法定义良好
new
方法正确初始化了LinkBackDfs
实例并设置了初始状态。
27-33
: 方法定义良好
empty
方法正确设置了结构体的空状态。
35-38
: 方法定义良好
move_to
方法正确更新了堆栈以从给定节点开始。
40-76
: 深度优先搜索实现良好,但逻辑复杂
next
方法正确实现了深度优先搜索,但逻辑复杂,需要仔细检查。建议确保所有边缘情况都已处理,并考虑添加单元测试以验证其行为。
48-74
: 验证边缘情况建议验证
next
方法中的边缘情况,并考虑添加单元测试以确保其行为正确。crates/mako/Cargo.toml (1)
94-94
: 依赖项添加良好添加
nohash-hasher
依赖项是合理的,建议验证其使用情况。Verification successful
依赖项添加良好
添加
nohash-hasher
依赖项是合理的,并且已在以下文件中使用:
crates/mako/src/module_graph/mod.rs
crates/mako/src/module_graph/link_back_dfs_visit.rs
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new dependency `nohash-hasher`. # Test: Search for the usage of `nohash-hasher` in the codebase. rg --type rust 'nohash-hasher'Length of output: 30
Script:
#!/bin/bash # Description: Verify the usage of the new dependency `nohash-hasher`. # Test: Search for the import statements of `nohash-hasher` in the codebase. rg --type rust 'extern crate nohash_hasher' -A 5 # Test: Search for references to `nohash-hasher` in the codebase. rg --type rust 'nohash_hasher::' -A 5Length of output: 1092
crates/mako/src/module_graph/mod.rs (1)
212-232
: 方法定义良好,但需要验证边缘情况和性能
get_dependencies_by_dfs
方法逻辑正确,但需要验证边缘情况和性能。建议添加单元测试和性能基准测试。crates/mako/src/generate/group_chunk.rs (16)
Line range hint
14-14
: 优化建议:使用HashSet
使用
HashSet
来跟踪访问的模块提高了依赖检查的效率。
Line range hint
15-15
: 优化建议:重构依赖处理逻辑将处理动态依赖和 worker 依赖的逻辑移到单独的函数中,提高了代码的可读性和维护性。
Line range hint
48-48
: 优化建议:使用 DFS 遍历使用深度优先搜索(DFS)来处理动态依赖关系是一个明智的选择,可以确保所有依赖关系都被正确处理。
Line range hint
49-49
: 优化建议:关注点分离将动态依赖和 worker 依赖的处理逻辑分开,使代码更清晰,易于维护。
Line range hint
68-68
: 优化建议:使用 DFS 遍历使用深度优先搜索(DFS)来处理 worker 依赖关系是一个明智的选择,可以确保所有依赖关系都被正确处理。
Line range hint
69-69
: 优化建议:关注点分离将 worker 依赖和动态依赖的处理逻辑分开,使代码更清晰,易于维护。
Line range hint
88-88
: 优化建议:集成新方法集成新的依赖处理方法,提高了函数的效率和可读性。
Line range hint
152-152
: 函数良好该函数逻辑清晰,未发现问题。
Line range hint
161-161
: 函数良好该函数逻辑清晰,未发现问题。
Line range hint
182-182
: 优化建议:集成新方法集成新的依赖处理方法,提高了函数的效率和可读性。
Line range hint
225-225
: 函数良好该函数逻辑清晰,未发现问题。
391-391
: 优化建议:使用HashSet
使用
HashSet
来跟踪依赖关系,提高了性能。
416-416
: 优化建议:简化依赖处理逻辑简化了依赖处理逻辑,使代码更清晰,易于维护。
Line range hint
438-438
: 优化建议:集成新方法集成新的依赖处理方法,提高了函数的效率和可读性。
Line range hint
491-491
: 优化建议:使用 DFS 遍历使用深度优先搜索(DFS)来处理依赖关系是一个明智的选择,可以确保所有依赖关系都被正确处理。
Line range hint
492-492
: 优化建议:关注点分离将依赖关系处理逻辑分开,使代码更清晰,易于维护。
f8c8591
to
5b32164
Compare
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.
Caution
Inline review comments failed to post
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
Files selected for processing (4)
- crates/mako/Cargo.toml (1 hunks)
- crates/mako/src/generate/group_chunk.rs (2 hunks)
- crates/mako/src/module_graph/link_back_dfs_visit.rs (1 hunks)
- crates/mako/src/module_graph/mod.rs (2 hunks)
Files skipped from review due to trivial changes (1)
- crates/mako/Cargo.toml
Files skipped from review as they are similar to previous changes (2)
- crates/mako/src/generate/group_chunk.rs
- crates/mako/src/module_graph/link_back_dfs_visit.rs
Additional comments not posted (5)
crates/mako/src/module_graph/mod.rs (5)
5-6
: 导入新依赖项引入了
LinkedHashSet
和BuildNoHashHasher
,这些依赖项适用于高效的数据结构管理。
16-17
: 新增模块导入新增的
link_back_dfs_visit
模块导入,表明有新的功能模块被引入。
212-213
: 获取模块索引通过
module_id
获取模块索引,如果模块不存在会引发 panic。建议在生产代码中处理这种情况以避免潜在的崩溃。请确保所有调用此方法的地方都能保证传入的
module_id
是存在的。
214-217
: 初始化 DFS 和依赖集合初始化
LinkBackDfs
和LinkedHashSet
,使用BuildNoHashHasher
提供高效的哈希功能。
227-231
: 收集依赖项将符合条件的依赖项收集到
LinkedHashSet
中,并最终转换为ModuleId
的向量返回。这种方法确保了依赖项的唯一性。
Comments failed to post (1)
crates/mako/src/module_graph/mod.rs
218-226: DFS 遍历和依赖检查
使用 DFS 遍历图并检查依赖项的
resolve_type
是否为DynamicImport
或Worker
。这种方法有效但可能需要进一步优化以提高性能。可以考虑在遍历过程中减少不必要的检查以提高性能。
5b32164
to
e918ef4
Compare
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
Files selected for processing (4)
- crates/mako/Cargo.toml (1 hunks)
- crates/mako/src/generate/group_chunk.rs (2 hunks)
- crates/mako/src/module_graph/link_back_dfs_visit.rs (1 hunks)
- crates/mako/src/module_graph/mod.rs (2 hunks)
Files skipped from review due to trivial changes (1)
- crates/mako/Cargo.toml
Files skipped from review as they are similar to previous changes (3)
- crates/mako/src/generate/group_chunk.rs
- crates/mako/src/module_graph/link_back_dfs_visit.rs
- crates/mako/src/module_graph/mod.rs
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
Files selected for processing (1)
- crates/mako/Cargo.toml (1 hunks)
Additional comments not posted (1)
crates/mako/Cargo.toml (1)
95-95
: 新依赖项nohash-hasher
的添加引入
nohash-hasher
作为新依赖项可能是为了优化哈希操作,提高性能。请确保该库与项目的其他部分兼容,并且确实有助于实现性能提升目标。请确认是否已在项目的其他部分正确使用了该库。
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
Files selected for processing (1)
- crates/mako/Cargo.toml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/mako/Cargo.toml
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1475 +/- ##
==========================================
+ Coverage 60.95% 61.06% +0.11%
==========================================
Files 125 126 +1
Lines 15057 15106 +49
==========================================
+ Hits 9178 9225 +47
- Misses 5879 5881 +2 ☔ View full report in Codecov by Sentry. |
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, codebase verification and nitpick comments (1)
crates/mako/src/module_graph/link_back_dfs_visit.rs (1)
1-11
: 建议:导入优化考虑将导入分组以提高可读性,例如将
std
库的导入放在一起,第三方库的导入放在一起。
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- crates/mako/src/generate/optimize_chunk.rs (1 hunks)
- crates/mako/src/module_graph/link_back_dfs_visit.rs (1 hunks)
- crates/mako/src/stats.rs (1 hunks)
Files skipped from review due to trivial changes (1)
- crates/mako/src/stats.rs
Additional comments not posted (6)
crates/mako/src/module_graph/link_back_dfs_visit.rs (5)
14-18
: 结构体定义合理
LinkBackDfs
结构体的定义合理,使用了LinkedHashMap
和FixedBitSet
来管理节点状态。
20-25
: 构造函数实现合理
new
方法初始化了DFS状态并移动到起始节点,逻辑清晰。
27-33
: 空构造函数实现合理
empty
方法为DFS创建了一个空的初始状态,使用了visit_map
来初始化节点状态。
35-38
: 方法实现合理
move_to
方法清空了堆栈并将起始节点插入,逻辑简单明了。
40-74
: 建议进行性能基准测试虽然
next
方法的性能看起来已经优化,但建议进行进一步的性能基准测试以确保其在大规模图形上的效率。crates/mako/src/generate/optimize_chunk.rs (1)
128-128
: 性能优化:使用back()
替代iter().last()
使用
back()
方法直接访问最后一个模块,提高了性能,尤其是在处理大数据集时。
Improve speed of chunks grouping on big project.
Before:
After:
Summary by CodeRabbit
Summary by CodeRabbit
新功能
nohash-hasher
,可能改善数据处理的性能或安全性。LinkBackDfs
结构体,增强图遍历能力,特别是在 CSS 处理方面。ModuleGraph
中新增get_dependencies_by_link_back_dfs
方法,支持基于深度优先搜索算法检索特定模块的依赖项。改进