Skip to content
This repository was archived by the owner on Dec 2, 2025. It is now read-only.

upgrade to lz4 v1.7.5 - #104

Closed
danielfree wants to merge 1 commit into
lz4:masterfrom
danielfree:master
Closed

upgrade to lz4 v1.7.5#104
danielfree wants to merge 1 commit into
lz4:masterfrom
danielfree:master

Conversation

@danielfree

@danielfree danielfree commented Jun 26, 2017

Copy link
Copy Markdown
Contributor

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

odaira commented Jun 26, 2017

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

@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

ijuma commented Aug 29, 2017

Copy link
Copy Markdown

Seems like lz4 1.8.0 should be the upgrade target.

@odaira

odaira commented Sep 11, 2017

Copy link
Copy Markdown
Member

@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

Copy link
Copy Markdown
Contributor Author

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

@odaira

odaira commented Sep 13, 2017

Copy link
Copy Markdown
Member

Yes, the master branch is ok.

@danielfree

danielfree commented Sep 20, 2017

Copy link
Copy Markdown
Contributor Author

@odaira plz have a look at #111, I'm going to close this pr.

@danielfree danielfree closed this Sep 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants