Skip to content

针对#issues6086导出整合包时内存溢出修复#6173

Open
2012hzy wants to merge 9 commits into
HMCL-dev:mainfrom
2012hzy:fix-export-modpack-oom
Open

针对#issues6086导出整合包时内存溢出修复#6173
2012hzy wants to merge 9 commits into
HMCL-dev:mainfrom
2012hzy:fix-export-modpack-oom

Conversation

@2012hzy

@2012hzy 2012hzy commented Jun 14, 2026

Copy link
Copy Markdown

问题:导出大型整合包时,HMCL 因 OutOfMemoryError 卡死(见 #6086)。
原因McbbsModpackExportTaskModrinthModpackExportTask 一次性将所有文件信息加载到内存生成 JSON,导致内存峰值过高
修复:改用 JsonWriter 流式写入临时文件,避免内存积累,并消除 ModrinthModpackExportTask 中重复的 API 调用和 I/O
测试:已完成编译测试,本地通过且功能正常
关联Fixes #6086

@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors both McbbsModpackExportTask and ModrinthModpackExportTask to stream JSON output directly using JsonWriter instead of constructing in-memory manifest objects. The review feedback highlights critical issues, including potential resource leaks from unclosed Files.walk streams and a potential NullPointerException when handling nullable Java versions. Additionally, the reviewer suggests refactoring repetitive loader version checks into loops to improve code readability and maintainability.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors both McbbsModpackExportTask and ModrinthModpackExportTask to write modpack manifests directly to temporary files using JsonWriter instead of serializing in-memory manifest objects. Feedback on these changes highlights several critical serialization bugs in the MCBBS export task, such as incorrect JSON keys (e.g., origins instead of origin, pluralized launch arguments, and supportedJavaVersions instead of supportJava), an incorrect type discriminator value, and missing default settings. Additionally, the reviewer notes that throwing a RuntimeException on remote file processing failures in the Modrinth export task could prematurely abort the export, and suggests refactoring ifPresent lambdas to standard loops or conditionals to avoid boilerplate try-catch blocks and RuntimeException wrapping.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the MCBBS and Modrinth modpack export tasks to stream JSON generation using JsonWriter to a temporary file, reducing memory overhead compared to the previous in-memory GSON serialization. However, several issues were identified in the review: using Files.walk instead of Files.walkFileTree introduces a severe performance regression by traversing blacklisted directories; writing explicit null values for optional fields like fileApi and summary may cause compatibility issues with strict parsers; and writing an empty origin array in the MCBBS manifest discards user-configured origin data.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/mod/mcbbs/McbbsModpackExportTask.java Outdated
@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors both McbbsModpackExportTask and ModrinthModpackExportTask to stream manifest and index JSON generation directly to a temporary file using JsonWriter instead of constructing large in-memory manifest objects. It also utilizes Files.walkFileTree for directory traversal. A critical issue was identified in ModrinthModpackExportTask where tryGetRemoteFile can throw an IOException that is no longer caught within the file visitor, which could cause the entire export task to fail on a single file error instead of logging a warning and continuing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the MCBBS and Modrinth modpack export tasks to stream JSON metadata directly to temporary files using JsonWriter instead of building full in-memory manifest objects, which optimizes memory usage. It also replaces stream-based file walking with Files.walkFileTree. Feedback on these changes includes addressing a potential NullPointerException in McbbsModpackExportTask when iterating over tokenized launch and Java arguments, and optimizing the whitelist lookup in ModrinthModpackExportTask by converting the whitelist List to a HashSet to avoid O(N) performance overhead during directory traversal.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the MCBBS and Modrinth modpack export tasks to stream JSON manifests directly to temporary files using JsonWriter instead of building large in-memory objects. It also updates directory traversal to use Files.walkFileTree. The review feedback highlights critical issues in both export tasks where directory traversal (preVisitDirectory and zip.putDirectory) prematurely skips subtrees when a whitelist is present, because directory paths themselves are not in the whitelist. The reviewer recommends allowing traversal of non-blacklisted directories and applying the whitelist filter only to files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors McbbsModpackExportTask and ModrinthModpackExportTask to stream their respective manifest files (mcbbs.packmeta and modrinth.index.json) directly to temporary files using JsonWriter instead of holding the entire manifest structures in memory. This is achieved by utilizing Files.walkFileTree to write files dynamically. A critical issue was identified in ModrinthModpackExportTask where enabling a disabled mod renames the file on disk but does not update the file reference, which can lead to a NoSuchFileException during subsequent operations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

/gemini review

@gemini-code-assist

Copy link
Copy Markdown
Contributor

你好!感谢你提交的关于修复大型整合包导出时 OutOfMemoryError 的 PR。这是一个非常棒的改进,通过使用 JsonWriter 进行流式写入,确实能有效降低内存占用。

不过,我注意到你在评论区多次重复输入了 /gemini review 指令。这可能会导致系统重复触发审核流程。如果你已经提交了代码并希望我进行审核,请耐心等待,或者你可以直接在 PR 中指出你希望我重点关注的代码部分。

目前我已经根据你提供的补丁进行了分析,你的修复方案在逻辑上是正确的,使用了 JsonWriterFiles.walkFileTree 来避免一次性加载所有文件信息到内存中,这非常符合处理大型数据集的最佳实践。代码结构清晰,且通过 finally 块确保了临时文件的清理,处理得非常稳妥。

如果后续有其他问题或需要进一步的反馈,请随时告知。

@2012hzy

2012hzy commented Jun 14, 2026

Copy link
Copy Markdown
Author

核心维护者们好,PR 中 CI 检查已全部通过。为了将 #6086 这个影响用户的 Bug 修复,这边的所有修改均已完成。请问是否还有其他地方需要调整?谢谢!

@2012hzy 2012hzy closed this Jun 15, 2026
@2012hzy 2012hzy reopened this Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 导出整合包时内存溢出且HMCL卡死,无任何提示

1 participant