lz4 / lz4-java

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

Support for Memory classes in lz4-java #106

Open niketh opened 7 years ago

niketh commented 7 years ago

Druid is a open source datastore which uses Lz4-java for fast compression. We are moving our implementation from ByteBuffer to Memory . Memory provides the following benefits over ByteBuffer

I was thinking of adding an extra interface which woud decompress memory and has an interface like this: decompress(Memory src, int srcOff, WritableMemory dest, int destOff, int destLen);

Would you be open to discussing such a change?

Cc: @leerho

niketh commented 7 years ago

@odaira Any thoughts on this? Do you think you would be okay with that change?

odaira commented 7 years ago

Is it sufficient to expose an interface like compress(long srcAddr, long srcLen, long destAddr, long maxDestLen), where srcAddr and destAddr are raw memory addresses?

For your reference, snappy-java supports larray through a similar interface called rawCompress.

niketh commented 7 years ago

@odaira Perfect , that would work. We don't expose the raw memory addresses in Memory. But i can work with @leerho to get that done.

Would you be open to accepting a pull request with the required interface change and implementation? compress(long srcAddr, long srcLen, long destAddr, long maxDestLen);

leerho commented 7 years ago

@niketh

It already does. For this purpose, Memory.getCumulativeOffset() does what you want. You must be careful to make sure the underlying native memory allocation is still valid, that the range of access is within the bounds of the underlying native memory allocation, and to make sure you close it when done. Forgetting to manage this properly could lead to difficult to debug seg-faults.

niketh commented 7 years ago

@odaira @leerho Perfect. Will create a PR with the changes then.

odaira commented 7 years ago

Yes, I am open to supporting compress/decompress methods that take raw memory addresses. I don't think lz4-java should directly support Memory at this moment because it is not a Java standard, but it should support any interfaces that are generic enough and that can unleash the true power of Memory.

BTW, please understand that I will not include any changes for Memory in the next version because they will break the JNI interface and because I would like to release the next version as soon as possible (hopefully this month). Such changes will go into the version after the next, which will probably get released sometime within this year.

leerho commented 7 years ago

@odaira

BTW, please understand that I will not include any changes for Memory in the next version because they will break the JNI interface

Do you mean you will not support the raw memory address API in the next version ...? When you are able to support raw memory addresses you will not need any references to Memory at all. We will be able to interface with your code just fine.

odaira commented 7 years ago

I mean I don't include the support for the raw memory address API in the next version (1.4.0), which will be released this month, but I will include it in version 1.5.0, which will be released after this month but sometime this year.