perf(instance): update modlist translate logic to improve retrieve speed #1106
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the mod list translation retrieval process to improve performance through concurrency control and enhanced caching. The changes focus on reducing redundant translation lookups and preventing resource exhaustion.
Key changes:
- Implements semaphore-based concurrency control to limit parallel mod processing tasks
- Refactors cache logic to use file names as keys instead of file paths, improving cache hit rate
- Consolidates cache persistence to a single write operation at the end of mod list retrieval
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src-tauri/src/lib.rs | Initializes and manages the mod translation cache at application startup |
| src-tauri/src/instance/helpers/mods/common.rs | Refactors cache structure, removes async file I/O functions, switches cache key from file path to file name |
| src-tauri/src/instance/constants.rs | Adds constants for cache expiry time and cache file name |
| src-tauri/src/instance/commands.rs | Implements semaphore for concurrency control, consolidates cache updates to single write operation |
| }; | ||
| let file_path = mod_info.file_path.to_string_lossy().to_string(); | ||
| let cache = load_local_mod_translations_cache(app).await; | ||
| let file_name = mod_info.file_name.clone(); |
There was a problem hiding this comment.
Unnecessary clone operation. The file_name field is already a String and is only used for lookups and logging. Consider borrowing it instead of cloning, or use &mod_info.file_name directly in subsequent operations.
src-tauri/src/instance/commands.rs
Outdated
| )); | ||
| for path in mod_paths { | ||
| let task = tokio::spawn(async move { get_mod_info_from_jar(&path).await.ok() }); | ||
| let permit = semaphore.clone().acquire_owned().await.unwrap(); |
There was a problem hiding this comment.
Using unwrap() on semaphore acquisition will panic if the semaphore is closed or if there's an error. Consider using proper error handling with ? operator or providing a meaningful error message.
src-tauri/src/instance/commands.rs
Outdated
| let permit = semaphore.clone().acquire_owned().await.unwrap(); | ||
| let task = tokio::spawn(async move { |
There was a problem hiding this comment.
Using unwrap() on semaphore acquisition will panic if the semaphore is closed or if there's an error. Consider using proper error handling with ? operator or providing a meaningful error message.
| let permit = semaphore.clone().acquire_owned().await.unwrap(); | |
| let task = tokio::spawn(async move { | |
| let task = tokio::spawn(async move { | |
| let permit = semaphore.clone().acquire_owned().await.ok()?; |
src-tauri/src/instance/constants.rs
Outdated
| @@ -1 +1,3 @@ | |||
| pub const INSTANCE_CFG_FILE_NAME: &str = "sjmclcfg.json"; | |||
| pub const CACHE_EXPIRY_HOURS: u64 = 24; | |||
There was a problem hiding this comment.
这两个 const 放在这就有点意义混淆了，建议增加 TRANSLATION_ 前缀
src-tauri/src/instance/commands.rs
Outdated
| )); | ||
| for path in mod_paths { | ||
| let task = tokio::spawn(async move { get_mod_info_from_jar(&path).await.ok() }); | ||
| let permit = semaphore.clone().acquire_owned().await.unwrap(); |
src-tauri/src/instance/commands.rs
Outdated
| let task = tokio::spawn(async move { get_mod_info_from_jar(&path).await.ok() }); | ||
| let permit = semaphore.clone().acquire_owned().await.unwrap(); | ||
| let task = tokio::spawn(async move { | ||
| info!("Processing mod: {}", path.display()); |
There was a problem hiding this comment.
log 信息请改为：Load mod info from jar: {}
下面那个请改为：
log::debug!("Load mod info from dir: {}", path.display())
（可以不用 use log，直接 log::info!, log::debug!，更明显说明这是 logger 提供的方法，否则 error! 容易有误解）
|
话说这个translation cache json大小大吗？ |
我装了五个大整合包后会有点大的，会到400KB |
会影响启动时间吗 有点好奇 |
基本没有捏 |
|
可以缓解 #1100 |
3 participants
Checklist
This PR is a ..
Related Issues
Description
Additional Context