-
Notifications
You must be signed in to change notification settings - Fork 82
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
feat(ssu): handle dependence changing while watching #1690
Conversation
Walkthrough此PR引入了对SSU(延迟依赖构建)状态的管理,定义了两个状态:FirstBuilding和Updating。通过这些状态的转换,决定SSU缓存的可用性,并在依赖发生变化时进行相应的处理,以提升启动速度。 Changes
|
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1690 +/- ##
==========================================
- Coverage 55.64% 55.45% -0.19%
==========================================
Files 173 173
Lines 17530 17588 +58
==========================================
- Hits 9754 9753 -1
- Misses 7776 7835 +59 ☔ 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: 1
🧹 Outside diff range and nitpick comments (9)
crates/mako/src/plugin.rs (2)
186-189
: 在 Plugin trait 中添加了新的生命周期方法这个新增的
after_update
方法为插件提供了在更新后执行操作的机会,与 PR 中提到的依赖变更处理功能相符。建议添加方法文档说明其具体用途和调用时机。建议添加如下文档注释:
+ /// 在更新完成后被调用,允许插件执行必要的清理或状态更新操作 + /// + /// # 参数 + /// * `compiler` - 编译器实例的引用,提供对编译上下文的访问 + /// + /// # 返回值 + /// 返回 Result<()>,表示操作是否成功 fn after_update(&self, _compiler: &Compiler) -> Result<()> { Ok(()) }
Line range hint
197-202
: NextBuildParam 结构体的设计合理该结构体包含了构建过程中所需的关键信息,字段的生命周期标注正确。但建议添加结构体级别的文档说明其用途。
建议添加如下文档注释:
+/// 包含下一次构建所需的参数 +/// +/// 该结构体携带了当前模块、下一个文件以及资源解析器的引用, +/// 用于在构建过程中追踪模块变更。 #[derive(Debug)] pub struct NextBuildParam<'a> { pub current_module: &'a ModuleId, pub next_file: &'a File, pub resource: &'a ResolverResource, }crates/mako/src/dev/update.rs (2)
334-335
: 优化:重复的克隆操作在第 334-335 行,您多次对
resolved_path
进行克隆:let dep_module_id = ModuleId::new(resolved_path.clone());考虑减少不必要的克隆操作,以提升性能。
338-338
: 建议:处理可能的错误情况在第 338 行的
Self::create_empty_module(&dep_module_id)
调用中,如果创建模块可能失败,请考虑添加错误处理,防止潜在的崩溃。crates/mako/src/plugins/ssu.rs (5)
69-74
: 建议为 SSUScanStage 添加文档注释为了提高代码的可读性和维护性,建议为枚举
SSUScanStage
添加文档注释,说明各个阶段FirstBuild
和Updating
的含义。
77-79
: 考虑使用原子类型替代 Arc<Mutex>对于简单的布尔标志,如
cache_valid
和will_full_rebuild
,使用Arc<AtomicBool>
可能比Arc<Mutex<bool>>
更高效,减少锁的开销和线程同步的性能损耗。
246-253
: 优化字符串处理以提高性能在构造
hmr_runtime
时,连续调用了to_string()
和多个replace()
方法。可以考虑使用一次性格式化或使用模板字符串来减少字符串拷贝和临时对象的创建,从而提高性能。
Line range hint
319-370
: 重构复杂逻辑以增强可读性在
next_build
方法中的匹配逻辑较为复杂,嵌套了多层条件判断。建议将复杂的逻辑拆分为多个小函数,或者添加详细的注释,以提高代码的可读性和可维护性。
Line range hint
559-588
: 注意动态生成的 JavaScript 代码的安全性在
runtime_plugins
方法中,直接将序列化的js_patch_map
和css_patch_map
插入到 JavaScript 代码中。请确保这些数据在序列化和插入过程中不会引入安全漏洞,例如避免可能的代码注入风险。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
crates/mako/src/dev/update.rs
(3 hunks)crates/mako/src/plugin.rs
(2 hunks)crates/mako/src/plugins/ssu.rs
(14 hunks)
🔇 Additional comments (6)
crates/mako/src/plugin.rs (1)
435-440
: PluginDriver 中的 after_update 实现符合预期
实现遵循了插件系统的一贯模式,正确处理了错误传播。代码结构清晰,无需修改。
crates/mako/src/dev/update.rs (3)
343-347
: 验证:next_build
方法的调用是否符合预期
在调用 self.context.plugin_driver.next_build
时,确保:
- 所有传入的参数都已正确初始化,特别是
next_file
是否已正确创建。 next_build
方法的执行不会引入性能瓶颈或阻塞。
258-260
:
注意:after_update
方法的错误处理
在 update
方法中新增了 self.context.plugin_driver.after_update(self)?
调用。请确保:
- 该方法可能返回的错误被正确处理,或者确认使用
?
运算符在此处是合适的。 - 理解
after_update
方法的副作用,确保不会对后续流程产生未预期的影响。
334-347
: 🛠️ Refactor suggestion
建议:在并发环境中确保插件调用的线程安全
在 build_by_modify
方法中,通过 par_iter
并行迭代时,您添加了对 self.context.plugin_driver.next_build
的调用。请注意:
- 确保
PluginDriver
及其插件实现是线程安全的,以避免数据竞争或其他并发问题。 - 验证
NextBuildParam
中传递的参数是否正确,如current_module
、next_file
和resource
是否在并发环境下安全使用。 - 考虑对
next_build
方法的返回值进行错误处理或结果验证,确保其执行成功。
crates/mako/src/plugins/ssu.rs (2)
142-149
: 检查多线程环境下的锁使用
在 in_building_stage
和 in_updating_stage
方法中,对 stage
字段进行加锁操作。请确保在多线程环境下,这些锁的使用不会导致死锁或资源竞争问题。
485-488
: 确保缓存验证逻辑的正确性
在 after_generate_chunk_files
方法中,如果 cache_valid
为 true
,函数将直接返回,跳过后续的文件生成步骤。请确认这符合预期行为,以避免可能的文件缺失或更新不完整的问题。
fn after_update(&self, compiler: &Compiler) -> Result<()> { | ||
if self.will_full_rebuild() { | ||
let files = self | ||
.dependence_node_module_files | ||
.iter() | ||
.map(|f| f.clone()) | ||
.collect::<Vec<File>>(); | ||
|
||
debug!("start to build after update"); | ||
let mut modules = compiler.build(files.clone())?; | ||
|
||
modules.extend(files.into_iter().map(|f| ModuleId::from(f.path))); | ||
|
||
transform_modules(modules.into_iter().collect(), &compiler.context)? | ||
} | ||
Ok(()) |
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.
处理 after_update
方法中的可能错误
在 after_update
方法中,调用了 transform_modules
,该方法可能返回错误。建议在调用后检查错误并进行适当的错误处理,以避免在运行时出现未捕获的异常。
一句话总结,
在没有模块 build 缓存的情况下,SSU 可以理解成,延迟依赖构建的方式来,提升启动速度的方案。
Summary by CodeRabbit
新功能
after_update
,允许插件在更新后执行必要操作。SSUScanStage
来管理扫描阶段,改进了缓存和重建逻辑。修复
文档