search

ning / compress

High-performance, streaming/chunking Java LZF codec, compatible with standard C LZF package
Other
254 stars 40 forks source link

[WIP] let lzf support ByteBuffer #47

Closed carryxyh closed 2 years ago

carryxyh commented 4 years ago

let lzf support ByteBuffer for encode.

carryxyh commented 4 years ago

@cowtowncoder

Pls help to review... I have to say that this is a bit complicated and requires more review and testing.

cowtowncoder commented 4 years ago

@carryxyh thanks, will try to find time to review this.

carryxyh commented 4 years ago

I need to say that my code is not elegant right now, but we need to make sure he is bug-free first. After this I will make further optimizations and adjustments...

In addition, this pr still has some code to submit, I just implemented the more critical VanillaChunkEncoder, UnsafeChunkEncoderBE, and UnsafeChunkEncoderLE.. Would u pls check this class first?

Thx..

cowtowncoder commented 4 years ago

On second part: "unsafe" variants are probably not relevant here as they just use couple of methods from sun.misc.Unsafe to do direct memory access, but this is not something usable or relevant when using ByteBuffer abstraction. So "vanilla" version is fine, although if possible it might be nice to just have same functionality for both cases. Big- vs Little-endian (BE/LEF) difference I am less sure about, but I think that as per:

https://stackoverflow.com/questions/26930066/bytebuffer-switch-endianness

you would only need to implement it one way and caller would then switch endian-ness if that matters. I guess it comes down to whether code uses any multi-byte (int / long) accessors; if not, endianness is irrelevant.

It has been a while since I worked on this code base so I think the most important thing really is to use unit tests: I assume it should be possible to extend and/or re-purpose existing tests for verification.

carryxyh commented 4 years ago

Thank you for your reply. As a result of the changes to ChunkEncoder, I have added several tryCompress abstract methods:

image

If only Vanilla needs to support ByteBuffer, how does theunsafe version of the encoder implement the two new tryCompress methods?

It seems that throwing exceptions is not an elegant way. How about transferring tryCompress from unsafe to Vanilla?

Currently I have completed the implementation of the tryCompress method and passed several unit tests.

To complete the whole pr, there are still many things to do. I need to add a lot of apis to LZFChunk and LZCEncoder.

cowtowncoder commented 4 years ago

I suspect unsafe should simply throw UnsupportedOperationException if methods were called on it.