search

lz4 / lz4-java

LZ4 compression for Java
Apache License 2.0
1.11k stars 252 forks source link

upgrade to lz4 v1.7.5 #104

Closed danielfree closed 7 years ago

danielfree commented 7 years ago

This pr basically upgrades lz4 to v1.7.5 from https://github.com/lz4/lz4. Some method name in JNI file is changed because of the original method being deprecated in lz4.c . Also the xxhash directory is no longer needed because lz4 package already contains the xxhash.c & xxhash.h .

odaira commented 7 years ago

Thanks much for your contribution. I'll work on your patch after I release a new version (1.4.0) soon. The proposed upgrade to lz4 1.7.5 will change the JNI interface, and we cannot delay the next release until we get contributions of liblz4-java.so for all the platforms. I'll release 1.5.0 with your patch soon after 1.4.0.

danielfree commented 7 years ago

@odaira Actually the upgrade will break some tests because the implementation of lz4.c is changed. But I believe the functionality is ok. I've looked through the code, it seems like the pure java implementations of Compressor/Decompressor also need update if we want to adhere to upstream change. Downgrade lz4 version can pass all tests.

Besides, is it better to have a git submodule depending on upstream lz4 git repo instead of manually copying the source code?

ijuma commented 7 years ago

Seems like lz4 1.8.0 should be the upgrade target.

odaira commented 7 years ago

@danielfree I looked into the code and confirmed that the native and the pure-Java compressors produced different outputs, probably because the new native compressor uses a different hash function. I don't think the pure-Java version needs to adhere to the upstream change because it is impractical as the upstream changes the behavior depending on the register size, endianness, etc. I think we should remove the test cases that assume both of the compressors produce the same output.

Also, I think it makes sense to use a git submodule. Are you going to work on that? If not, I'll do that.

danielfree commented 7 years ago

@odaira what branch are we going to base on? is master branch ok?

odaira commented 7 years ago

Yes, the master branch is ok.

danielfree commented 7 years ago

@odaira plz have a look at https://github.com/lz4/lz4-java/pull/111, I'm going to close this pr.