Fix: #4958 模组检查更新 #5029
Merged
Fix: #4958 模组检查更新 #5029
+47
−40
Conversation
This file contains hidden or 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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #4958 by addressing an Out-Of-Memory (OOM) problem in the mod update checking functionality. The original implementation retained all historical versions of all mods during update checks, causing excessive memory usage.
Changes:
- Refactored
ModUpdateto store a single candidate version instead of a list of all newer versions - Consolidated mod update checking logic to reduce the number of concurrent tasks and memory footprint
- Updated UI code to reference the single candidate version instead of selecting from a list
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| HMCLCore/src/main/java/org/jackhuang/hmcl/mod/LocalModFile.java | Changed ModUpdate to store single candidate version; checkUpdates() now returns only the most recent version |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModCheckUpdatesTask.java | Refactored to create one task per mod (instead of per mod-repository pair) and select the newest update internally |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModUpdatesPage.java | Updated references from getCandidates().get(0) to getCandidate() |
Comments suppressed due to low confidence (1)
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModCheckUpdatesTask.java:81
- The task returns
falseforisRelyingOnDependents()but still callsisDependentsSucceeded()logic implicitly through the result collection inexecute(). This is inconsistent with the pattern seen in other tasks. WhenisRelyingOnDependents()isfalse, tasks typically need to explicitly check dependent task success. However, since the current implementation filters null results (which would be returned on failure), this works correctly but could be clearer. Consider adding a comment explaining why returningfalseis safe here, or ensure the pattern is consistent with how other tasks handle dependent failures.
public boolean isRelyingOnDependents() {
return false;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
原来的码太狗屎了，竟然会保留全部模组的全部历史版本待查，怪不得这么容易 OOM
该 PR 必须在 #5026 后合并，否则会 DOS 服务器