feat(experimental): dev simple speed up #1186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough总体变更概述：在 Changes
Poem
Note Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://coderabbit.ai 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 Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
crates/mako/src/plugins/ssu.rs
Outdated
Comment on lines
167
to
235
| fn load(&self, param: &PluginLoadParam, context: &Arc<Context>) -> Result<Option<Content>> { | ||
| if param.file.path.starts_with(SSU_ENTRY_PREFIX) { | ||
| let path_string = param.file.path.to_string_lossy().to_string(); | ||
| let start = SSU_ENTRY_PREFIX.len(); | ||
| let path = PathBuf::from(path_string.as_str()[start..].to_string()); | ||
|
|
||
| let mut require_externals = context | ||
| .config | ||
| .externals | ||
| .iter() | ||
| .map(|ext| format!("require('{}');", ext.0)) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| require_externals.sort(); | ||
|
|
||
| let mut reverse_require = self | ||
| .cached_state | ||
| .lock() | ||
| .unwrap() | ||
| .reversed_required_files | ||
| .iter() | ||
| .map(|f| format!("require('{}')", f)) | ||
| .collect::<Vec<_>>(); | ||
| reverse_require.sort(); | ||
|
|
||
| let port = &context.config.hmr.as_ref().unwrap().port.to_string(); | ||
| let host = &context.config.hmr.as_ref().unwrap().host.to_string(); | ||
| let host = if host == "0.0.0.0" { "127.0.0.1" } else { host }; | ||
| let hmr_runtime = include_str!("../runtime/runtime_hmr_entry.js") | ||
| .to_string() | ||
| .replace("__PORT__", port) | ||
| .replace("__HOST__", host); | ||
|
|
||
| let content = format!( | ||
| r#" | ||
| require("virtual:C:/node_modules/css/css.css"); | ||
| let patch = require._su_patch(); | ||
| console.log(patch); | ||
| {} | ||
| module.export = Promise.all( | ||
| patch.map((d)=>__mako_require__.ensure(d)) | ||
| ).then(()=>{{ | ||
| {} | ||
| {} | ||
| return require("{}"); | ||
| }}, console.log); | ||
| "#, | ||
| require_externals.join("\n"), | ||
| hmr_runtime, | ||
| reverse_require.join("\n"), | ||
| path.to_string_lossy() | ||
| ); | ||
|
|
||
| debug!("entry content:\n{}", content); | ||
|
|
||
| return Ok(Some(Content::Js(JsContent { | ||
| content, | ||
| is_jsx: false, | ||
| }))); | ||
| } | ||
|
|
||
| if param | ||
| .file | ||
| .path | ||
| .starts_with("virtual:C:/node_modules/css/css.css") | ||
| { | ||
| return Ok(Some(Content::Css("._mako_mock_css { }".to_string()))); | ||
| } | ||
| Ok(None) |
There was a problem hiding this comment.
在 load 方法中，处理了特定前缀的文件路径。这种硬编码的路径可能会导致在不同环境下的兼容性问题。建议使用配置文件或环境变量来管理这些路径。
Comment on lines
+238
to
+310
| fn next_build(&self, next_build_param: &NextBuildParam) -> bool { | ||
| let from: CodeType = next_build_param | ||
| .current_module | ||
| .id | ||
| .contains("/node_modules/") | ||
| .into(); | ||
| let to = next_build_param.next_file.is_under_node_modules.into(); | ||
|
|
||
| debug!( | ||
| "{} -> {}", | ||
| next_build_param.current_module.id, | ||
| next_build_param | ||
| .next_file | ||
| .pathname | ||
| .to_string_lossy() | ||
| .to_string() | ||
| ); | ||
|
|
||
| match (from, to) { | ||
| (CodeType::SourceCode, CodeType::Dependency) => { | ||
| if let ResolverResource::Resolved(resolved) = &next_build_param.resource { | ||
| self.dependence_node_module_files | ||
| .insert(next_build_param.next_file.clone()); | ||
|
|
||
| let path_name = next_build_param | ||
| .next_file | ||
| .path | ||
| .to_string_lossy() | ||
| .to_string(); | ||
|
|
||
| let version = resolved | ||
| .0 | ||
| .package_json() | ||
| .and_then(|p| p.raw_json().get("version")) | ||
| .map_or("0.0.0".to_string(), |v| { | ||
| v.as_str().unwrap_or("0.0.0").to_string() | ||
| }); | ||
|
|
||
| self.current_state | ||
| .lock() | ||
| .unwrap() | ||
| .cached_boundaries | ||
| .insert(path_name, version); | ||
|
|
||
| let scanning = *self.scanning.lock().unwrap(); | ||
| !scanning | ||
| } else { | ||
| true | ||
| } | ||
| } | ||
| (CodeType::Dependency, CodeType::SourceCode) => { | ||
| debug!( | ||
| "{} -> {}", | ||
| next_build_param.current_module.id, | ||
| next_build_param.next_file.pathname.to_string_lossy() | ||
| ); | ||
|
|
||
| self.current_state | ||
| .lock() | ||
| .unwrap() | ||
| .reversed_required_files | ||
| .insert( | ||
| next_build_param | ||
| .next_file | ||
| .pathname | ||
| .to_string_lossy() | ||
| .to_string(), | ||
| ); | ||
| true | ||
| } | ||
| _ => true, | ||
| } | ||
| } |
There was a problem hiding this comment.
next_build 方法中的逻辑较为复杂，涉及多个条件判断和状态转换。建议添加详细的日志记录和错误处理，以便于问题追踪和调试。
Comment on lines
+426
to
+488
| fn build_start(&self, context: &Arc<Context>) -> Result<Option<()>> { | ||
| if let Some(content) = self.load_cached_state(context) { | ||
| let mut state = self.cached_state.lock().unwrap(); | ||
| *state = content; | ||
| } | ||
|
|
||
| self.current_state.lock().unwrap().config_hash = Self::config_hash(&context.config); | ||
|
|
||
| Ok(None) | ||
| } | ||
|
|
||
| fn runtime_plugins(&self, _context: &Arc<Context>) -> Result<Vec<String>> { | ||
| if *self.enabled.lock().unwrap() { | ||
| let cache = self.cached_state.lock().unwrap(); | ||
|
|
||
| let code = format!( | ||
| r#" | ||
| requireModule._su_patch = function(){{ | ||
| var js_patch = {}; | ||
| var css_patch = {}; | ||
| for(var key in js_patch) {{ | ||
| chunksIdToUrlMap[key] = js_patch[key]; | ||
| }} | ||
| for(var key in js_patch) {{ | ||
| cssChunksIdToUrlMap[key] = css_patch[key]; | ||
| }} | ||
| return Object.keys(js_patch).sort(); | ||
| }} | ||
| "#, | ||
| serde_json::to_string(&cache.js_patch_map).unwrap(), | ||
| serde_json::to_string(&cache.css_patch_map).unwrap(), | ||
| ); | ||
|
|
||
| Ok(vec![code]) | ||
| } else { | ||
| let cg = _context.chunk_graph.read().unwrap(); | ||
|
|
||
| cg.get_chunks() | ||
| .into_iter() | ||
| .filter(|c| c.chunk_type == ChunkType::Sync) | ||
| .for_each(|c| { | ||
| println!("chunk: {}", c.filename()); | ||
| }); | ||
|
|
||
| Ok(vec![r#" | ||
| requireModule._su_patch = function(){ | ||
| var js_patch = { | ||
| "node_modules": "node_modules.js" | ||
| }; | ||
| var css_patch = { | ||
| "node_modules": "node_modules.css" | ||
| }; | ||
| for(var key in js_patch) { | ||
| chunksIdToUrlMap[key] = js_patch[key]; | ||
| } | ||
| for(var key in js_patch) { | ||
| cssChunksIdToUrlMap[key] = css_patch[key]; | ||
| } | ||
| return ["node_modules"]; | ||
| }"# | ||
| .to_string()]) | ||
| } | ||
| } |
There was a problem hiding this comment.
runtime_plugins 方法中，生成的代码片段使用了大量的硬编码字符串。这可能会导致维护困难和错误。建议将这些字符串提取为常量或配置项。
crates/mako/src/plugins/ssu.rs
Outdated
Comment on lines
105
to
109
| fn write_current_cache_state(&self, context: &Arc<Context>) { | ||
| let cache_file = context.root.join("node_modules/.cache_mako/meta.json"); | ||
| let cache = self.current_state.lock().unwrap(); | ||
| fs::write(cache_file, serde_json::to_string(&*cache).unwrap()).unwrap(); | ||
| } |
There was a problem hiding this comment.
write_current_cache_state 方法中直接使用了 unwrap()，这在生产环境中可能导致程序意外崩溃。建议改为返回错误，让调用者处理。
- fs::write(cache_file, serde_json::to_string(&*cache).unwrap()).unwrap();
+ let cache_data = serde_json::to_string(&*cache)?;
+ fs::write(cache_file, cache_data)?;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| fn write_current_cache_state(&self, context: &Arc<Context>) { | |
| let cache_file = context.root.join("node_modules/.cache_mako/meta.json"); | |
| let cache = self.current_state.lock().unwrap(); | |
| fs::write(cache_file, serde_json::to_string(&*cache).unwrap()).unwrap(); | |
| } | |
| fn write_current_cache_state(&self, context: &Arc<Context>) -> Result<(), Box<dyn std::error::Error>> { | |
| let cache_file = context.root.join("node_modules/.cache_mako/meta.json"); | |
| let cache = self.current_state.lock().unwrap(); | |
| let cache_data = serde_json::to_string(&*cache)?; | |
| fs::write(cache_file, cache_data)?; | |
| Ok(()) | |
| } |
Comment on lines
+135
to
+162
| fn modify_config(&self, config: &mut Config, _root: &Path, _args: &Args) -> Result<()> { | ||
| for p in config.entry.values_mut() { | ||
| *p = PathBuf::from(format!("{SSU_ENTRY_PREFIX}{}", p.to_string_lossy())); | ||
| } | ||
|
|
||
| config.code_splitting = Some(CodeSplittingStrategy::Advanced(OptimizeChunkOptions { | ||
| min_size: 0, | ||
| groups: vec![ | ||
| OptimizeChunkGroup { | ||
| name: "node_modules".to_string(), | ||
| allow_chunks: OptimizeAllowChunks::All, | ||
| min_chunks: 0, | ||
| min_size: 0, | ||
| max_size: usize::MAX, | ||
| priority: 10, | ||
| test: Regex::new(r"[/\\]node_modules[/\\]").ok(), | ||
| }, | ||
| OptimizeChunkGroup { | ||
| name: "common".to_string(), | ||
| min_chunks: 0, | ||
| // always split, to avoid multi-instance risk | ||
| min_size: 1, | ||
| max_size: usize::MAX, | ||
| priority: 0, | ||
| ..Default::default() | ||
| }, | ||
| ], | ||
| })); |
There was a problem hiding this comment.
Tip
Codebase Verification
在 optimize_hot_update_chunk 和 get_optimize_chunk_options 方法中，都有检查 config.code_splitting 是否为 None 的逻辑。如果 config.code_splitting 为 None，这些方法将不会继续执行相关的优化逻辑。
然而，在 modify_config 方法中，config.code_splitting 被设置为一个特定的策略（CodeSplittingStrategy::Advanced），这意味着在调用 modify_config 后，config.code_splitting 不会是 None。
因此，这种修改可能会影响非 SSU 模式下的配置，因为 config.code_splitting 被强制设置为一个特定的策略，而不是保持原来的状态。
需要注意的代码位置：
crates/mako/src/plugins/ssu.rs中的modify_config方法。
Analysis chain
modify_config 方法中修改了配置以支持 SSU。请确保这些更改不会影响到非 SSU 模式下的构建配置。
根据 rg 的输出，config.code_splitting 在以下文件和位置被使用：
crates/mako/src/generate/optimize_chunk.rs中的optimize_hot_update_chunk方法。crates/mako/src/generate/optimize_chunk.rs中的get_optimize_chunk_options方法。
我们需要进一步检查这些方法的上下文，以确定 modify_config 方法中的更改是否会影响非 SSU 模式下的配置。
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查非 SSU 模式下的配置是否受到影响
rg --type rust 'config.code_splitting' --context 5
Length of output: 2825
Script:
#!/bin/bash
# 检查 optimize_hot_update_chunk 方法的上下文
rg --type rust 'fn optimize_hot_update_chunk' --context 10 crates/mako/src/generate/optimize_chunk.rs
# 检查 get_optimize_chunk_options 方法的上下文
rg --type rust 'fn get_optimize_chunk_options' --context 10 crates/mako/src/generate/optimize_chunk.rs
Length of output: 1831
sorrycc
requested changes
May 30, 2024
sorrycc
approved these changes
May 31, 2024
This was referenced Sep 12, 2024
This was referenced Sep 24, 2024
This was referenced Oct 10, 2024
This was referenced Nov 5, 2024
This was referenced Nov 29, 2024
This was referenced Dec 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SSU experimental implement
enabled only when
--watchmode is on and env variable setSSU=truealpha testing with a few internal teams
then main process
modify_configevent to change entry to SSU's entry, to set chunkMap patch, external Patch and HMRnext_buildto store all the dependencies and defer dependence module build.after_buildto determine whether continue build dependence or just use SSU cached chunk filesafter_generate_chunk_filesto gather all the dependences's chunk, this info will used in SSU entry and runtimefuture plan
virtual:C:/node_modules/css/css.csshackSummary by CodeRabbit
新功能
Cargo.toml文件中添加了dashmap依赖项。Hash和Eq实现。Hash和PartialEq用于File。fn parse_path(path: &str) -> Result<(PathName, Search, Params, Fragment)> {。优化
Compiler实现，引入了基于plugin_driver的条件逻辑。插件
plugin模块中添加了NextBuildParam。BuildTasksError的错误处理。Compiler实现，引入了基于plugin_driver的条件逻辑。性能优化
Compiler实现中关于日志记录的部分。配置
Hash特性。其他
DevServer实现中更改了对port值的处理。