Skip to content
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

Speedup backups slightly + max backup folder size config #100

Merged
merged 12 commits into from
Sep 13, 2024

Conversation

Lyfts
Copy link
Member

@Lyfts Lyfts commented Aug 2, 2024

  1. Switches the zip implementation from the native java one to Commons Compress which 1.7.10 ships with. After a lot of testing it showed a decent speed improvement, no clue why though since they use the same compression algorithm.

This is 10 runs on compression level 1 & DEFLATE,
The original folder was 3.65GB which was artificially inflated using copied region files.

Run # New Time (Commons Compress) Old Time (Native Java)
1 2:20 min 2:47 min
2 2:20 min 2:53 min
3 2:24 min 2:56 min
4 2:22 min 2:45 min
5 2:22 min 2:48 min
6 2:22 min 2:49 min
7 2:18 min 2:56 min
8 2:23 min 2:59 min
9 2:21 min 2:53 min
10 2:22 min 2:51 min

Both of them ended up with a final compressed size of 3.18GB and the new implementation was between 12 - 21% faster.

Here is another 3 runs also on level 1 & DEFLATE but with an uncompressed size of 10.6GB also artificially inflated in the same manner.

Run # New Time (Commons Compress) Old Time (Native Java)
1 7:04 min 8:20 min
2 7:24 min 8:25 min
3 7:18 min 8:14 min

Final compressed size was 9.42GB for both and it's now between 10 - 14% faster.
Overall this is a very small sample size and only tested on one machine so YMMW but I think that the change is warranted regardless.

  1. Adds a config option to define a maximum size (in GB) for the backup folder rather than amount of backups. If the size exceeds the amount defined in config it will delete backups until under that size. This takes priority over the normal backup amount config and will be disabled by default.

  2. Readds the option to not have backups with a custom name (through /backup start <name>) deleted, which I removed in Fix backups not getting deleted under certain circumstances #48, but with a less awful implementation this time. This is also disabled by default.

@Lyfts Lyfts requested a review from a team August 2, 2024 22:51
@Glease
Copy link
Contributor

Glease commented Aug 27, 2024

try reenable biased locking if you are not on java 8. the builtin java thing goes through the default deflater and uses synchronized. neither of these are free.

@Lyfts
Copy link
Member Author

Lyfts commented Sep 9, 2024

Just for note #112 is (hopefully) fixed, but I chose to do it in #101 as ThreadBackup is almost rewritten there and it would cause far too much conflict between the branches if done here.

Copy link
Contributor

@Ethryan Ethryan left a comment

Choose a reason for hiding this comment

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

Code looks good, my only thoughts are about @Glease comment from above. Otherwise this looks good.

@Dream-Master Dream-Master enabled auto-merge (squash) September 13, 2024 05:37
@Dream-Master Dream-Master merged commit 9b013ce into master Sep 13, 2024
1 check passed
@Dream-Master Dream-Master deleted the backup-improvement branch September 13, 2024 05:41
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.

4 participants