On 2/16/18, 2:13 PM, David Lloyd wrote:
It would be convenient to be able to inflate/deflate a direct or heap
byte buffer without having to copy it through an array first. For my
Friday mini-project this week, I've decided to take a crack at this.
The attached patch is the result. Would anyone be interested in
reviewing and maybe sponsoring this change? It would be really great
to get it in to JDK 11.
The patch includes a modification to FlaterTest to run through
permutations of using direct and heap byte buffers. Though I couldn't
get jtreg working, I did compile and run the test by hand and it seems
to pass; the build also works fine with the new code.
Extra thanks to Martin Balao for pointing me towards mapfile-vers when
I couldn't figure out why I was getting UnsatisfiedLinkError. That
one was not obvious.
Hi David,
Thanks for taking this one :-) some comments here.
(1) I would assume you might have to do more for ByteBuffer, something like
if (bf.isDirect()) {
// GetDirectBufferAddress
...
} else if (bf.hasArray()) {
byte[] array = bf.getArray();
do(bf.getArray(), offset + pos, pos - limit);
...
} else {
// probably still have to copy the bytes out of ByteBuffer
...
}
btw, any reason go unsafe to get the byte[]? instead of
ByteBuffer.getArray()?
(2) It might be a concerned that you have to warp the input byte[] every
time
the inflate/deflate(byte[]...) is called. (Yes, I'm constantly
hearing people
complain that you have to wrap the byte[] to ByteBuffer to use
CharsetEn/
Decoder, or even the implementation detail inside StringCoder),
and it might
be taken as a "regression". So it might be desired to wire the
"bf.hasArray())
path inside de/encode(ByteArray) back to de/encode(byte[], int,
int), to avoid
the "unnecessary" ByteBuffer wrap.
(3) Same "wrap concern" argument might go to the setInput(bye[] ...) as
well.
I'm not sure if it is worth keeping both byte[]/int/int and
ByteBuffer as the
"input" field of In/Deflater.
(4) assume we keep the "wrap" approach. It appears ByteBuffer.wrap()
does check
the byte[]/off/len and throw an IndexOutOfBoundsException. So it
might be
better to take advantage of that check.
(5) Deflater.input need to be initialized to a non-null value.
Btw ZipUtil.defaultBuf needs to be "direct"?
(6) It might be desired to have some jmh measure to make sure byte[]
case does
not have regression.
Thanks,
Sherman
