-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add Zstd/LZ4 support for region files (1.15 edition) #2814
Conversation
Zstd using zstd-jni: https://github.com/luben/zstd-jni LZ4 using lz4-java: https://github.com/lz4/lz4-java New compression only happens when a chunk is saved, so running a force-upgrade may be desirable. New config option for the compression method to use. There's a new option in PaperConfig (heavy compression) for Zstd which defaults to false. Since the patch hooks Mojang's versioning system for new compression methods, it should be internally consistent, easily-updatable, and reversible with a force-upgrade. This will likely conflict with plugins and programs expecting region files to be compressed using the current standard.
Zstd using zstd-jni: https://github.com/luben/zstd-jni LZ4 using lz4-java: https://github.com/lz4/lz4-java New compression only happens when a chunk is saved, so running a force-upgrade may be desirable. New config option for the compression method to use. There's a new option in PaperConfig (heavy compression) for Zstd which defaults to false. Since the patch hooks Mojang's versioning system for new compression methods, it should be internally consistent, easily-updatable, and reversible with a force-upgrade. This will likely conflict with plugins and programs expecting region files to be compressed using the current standard.
Dictionary coming soon, as soon as I train one. This is the rest of the implementation completed. Due to some updates in the Zstd library, this version should be even faster than the previous implementation! |
Dictionary is done and I made some suggested corrections. |
Test results Byte array decompression can be made more efficient by including the uncompressed size in the region/chunk headers, but using both byte arrays and adding the new header would require modification that is beyond the scope of this patch. Perhaps a future patch. Zlib stream compression levels: Zstd stream compression levels: LZ4: Since 1.15 relies on streams, I decided not to fight it and let the ~15us penalty for streams over byte arrays go, as converting to a byte array would require more code, and thus be less maintainable down the line. tl;dr from Zlib (current) to Zlib (new, default level) would mean:
from Zlib (new, default level) to Zstd (new, default level, with the included dictionary) would mean:
|
How about a versioned dictionary? With future Minecraft versions it might be useful to update the dictionary. |
Spigot-Server-Patches/0420-Add-Zstd-LZ4-support-for-region-files-1.15-edition.patch
Outdated
Show resolved
Hide resolved
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.
I just wanna clarify here,what's the behavior of the zstd dicts if we bump? Do we have to keep the existing dict for decompression chunks or is the dict saved/whatever with the chunk data?
+ if (regionCompressionMethod == RegionCompressionType.ZSTD) { | ||
+ log("Using level " + regionCompressionZstdLevel + " for Zstd compression." + (regionCompressionZstdLevel >= 14 ? " (Make my PC hurt)" : "")); | ||
+ } | ||
+ regionCompressionZlibLevel = getInt("settings.region-compression.zlib-level", 5); |
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.
should default from the field, same for the one above too
existing dict needs to be kept, only the dict version is stored on chunk data |
Any news about this? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Closed in favor of #5029 |
Zstd using zstd-jni: https://github.com/luben/zstd-jni
LZ4 using lz4-java: https://github.com/lz4/lz4-java
Added new command-line option, --forceWrite, which will forcibly write the chunk/flush to disk regardless of whether or not the chunk has actually changed in a --forceUpgrade.
New compression only happens when a chunk is saved, so running a --forceUpgrade AND --forceWrite may be desirable.
New config option for the compression method to use.
There's a new option in PaperConfig (compression level) for Zstd which defaults to 6.
There's a new option in PaperConfig (compression level) for Zlib which defaults to 5.
Since the patch hooks Mojang's versioning system for new compression methods, it
should be internally consistent, easily-updatable, and reversible with a --forceUpgrade AND a --forceWrite.
This will likely conflict with plugins and programs expecting region files to be compressed using the current standard.