[JDK-6341887] Patch: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

[Xueming Shen]

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