-
Notifications
You must be signed in to change notification settings - Fork 774
feat: hard link instead of copying #4586
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
base: main
Are you sure you want to change the base?
Conversation
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.
有很大问题。
private static volatile boolean hardLink = true; | ||
|
||
private static final Object obj = new Object(); | ||
private static final ConcurrentHashMap<Path, Object> pathMap = new ConcurrentHashMap<>(); |
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.
我完全没有看懂你这里做 pathMap 的意义是什么。
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.
确保线程安全. 这个函数是并发的, 且实际测试也确实遇见了多次同时多个文件写入一个文件的情况, 出现了前面Files.exists(destFile)
判断文件不存在, 后面Files.createLink
又报错FileAlreadyExistsException
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.
最好是在创建任务时就去重, 但是copyFile引用的地方太多了, 改动太大可能不稳定, 也没有线程安全的弱引用Map, 可以移除不再使用的路径, 只添加不删除, 下载一个整合包会留下近4000字符串的引用, 这算内存泄漏了. 只能用pathMap
保证有复制到destFile
任务正在运行时, 其他任务退出
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.
确保线程安全. 这个函数是并发的, 且实际测试也确实遇见了多次同时多个文件写入一个文件的情况, 出现了前面
Files.exists(destFile)
判断文件不存在, 后面Files.createLink
又报错FileAlreadyExistsException
这样实现依然阻止不了多进程处理时出现问题的情况,所以不应该使用这样的方式来解决。
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.
确保线程安全. 这个函数是并发的, 且实际测试也确实遇见了多次同时多个文件写入一个文件的情况, 出现了前面
Files.exists(destFile)
判断文件不存在, 后面Files.createLink
又报错FileAlreadyExistsException
这样实现依然阻止不了多进程处理时出现问题的情况,所以不应该使用这样的方式来解决。
多进程确实没什么办法, 只能靠try-catch丢掉错误了
} catch (FileAlreadyExistsException e) {
LOG.warning(e.toString());
return;
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.
- 不应该使用
Map
防止重复,请安全地处理FileAlreadyExistsException
。 - 不应该使用类似
LOG.warning(e.toString())
这样的代码记录异常,这样无法记录异常栈。请使用LOG.warning("message", e)
记录异常。
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.
不应该使用
Map
防止重复,请安全地处理FileAlreadyExistsException
。不应该使用类似
LOG.warning(e.toString())
这样的代码记录异常,这样无法记录异常栈。请使用LOG.warning("message", e)
记录异常。
最好能够尝试在创建线程时去重, 尤其是在assets链接到cache时. 去掉判断后, 一次下载就有4处FileAlreadyExistsException
HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/FileUtils.java
Outdated
Show resolved
Hide resolved
使用硬链接会导致修改一处文件时所有文件跟着变化,所以不应该成为 |
HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/FileUtils.java
Outdated
Show resolved
Hide resolved
下载的文件有 |
缓存文件使用硬链接代替复制
以下是采用整合包 FO fabric 1.21.1 测试结果
可见, 能节省90%+的空间