Dynamic compression for world data - #90
Conversation
RecursivePineapple
added
Enhancement
|
Ping me when this is ready, then I'll review |
RecursivePineapple
changed the title
| .order(ByteOrder.LITTLE_ENDIAN); | ||
| } | ||
| } | ||
| case LZ4 -> { |
There was a problem hiding this comment.
Why not LZ4FrameInputStream/LZ4FrameOutputStream?
There was a problem hiding this comment.
I tried using frames but it just complicated the code too much. You don't need frames anyways, they just add compression length metadata and checksums. All of the LZ4 compressors are compatible with each other, so you don't need to worry about compressing with one and decompressing with another.
There was a problem hiding this comment.
Doesn't seem too complicated to me, unless I missed something. I have a version here: e7f31ba
Although I don't know how it is allocation wise, it seems simpler to me - that's the only reason I asked, seems like less code.
|
|
||
| public static byte[] saveTag(NBTTagCompound tag, boolean compress) throws IOException { | ||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
| private static int getTagSizeEstimate(NBTBase tag) { |
There was a problem hiding this comment.
I assume this is to prevent allocations when the stream grows? Can't we use some generous fixed number to avoid all this complexity? Like, take an average chunk size, multiply it by 2 and hardcode that? Should give most of the benefit with a lot less work.
There was a problem hiding this comment.
You're right, it is used to prevent reallocations of the backing byte array. I added it because I don't want to make oversized arrays - past a certain point (a few MB, depending on the JVM and its GC config), the array is immediately put into the old generation and causes more memory pressure.
I could remove the method and use a fixed initial size, but it doesn't cost anything other than a bit of code complexity. This method barely shows up on profiles. Most chunks/cubes are nowhere near the theoretical limit, since they don't have any tiles. A newly generated cube might be anywhere from 16kb to 48kb, depending on which nibble arrays are initialized.
Another benefit is that I can guarantee the BAOS won't reallocate its backing array (barring any BAOS code changes) because it won't ever need to write past its capacity, which should also help reduce object churn. Reallocating a small array is fine, but larger ones will quickly use up a good amount of memory. Doubly so, since this method is called hundreds of times a second.
I looked into pooling the byte array but it wasn't worth it. It was too tricky to pull it out of the BAOS and put it back in as needed. I also looked into raw memory allocations to avoid the GC pressure but they had a big performance hit due to the indirection, a pure java array is easier for the JIT to optimize.
There was a problem hiding this comment.
Eh, just seems like more complexity than it's worth.
If we keep it, can you please get rid of the MutableInt though? Normal loop + int is cheaper.
| CompressedStreamTools.write(tag, dos); | ||
| } | ||
|
|
||
| return ByteBuffer.wrap(nos.toByteArray()) |
There was a problem hiding this comment.
Maybe something like the ByteBufferOutputStream I have here e7f31ba to minimize allocations?
There was a problem hiding this comment.
Yeah that could work well
3 participants
Summary
Chunk/cube saving unconditionally uses GZIP for compression currently. This PR adds the ability for data to be compressed with different algorithms. Currently, only LZ4 is implemented. LZ4 was chosen because it is extremely fast, which eliminates most overhead when loading or saving world data. The algorithm is configurable so that server owners can tweak it or disable it entirely. The compression level can be changed at-will, and the world will be re-compressed with the new format as chunks and cubes are re-saved. I will also add something that re-compresses rarely used chunks/cubes when players haven't visited them in several IRL days.
LZ4 compression was also added to cube data, since CC was uploading over 10MB/s to players as they moved. This should significantly reduce network usage while keeping overhead low. We previously used GZIP to compress packets, but its overhead made the game unplayable.
Checklist