Review Request for 6751338: ZIP inflater/deflater performance

[Dave Bristor]

The webrev looks fine.  I have only this minor comment:

* Inflater.c:
A minor suggestion: In inflateBytes, cases Z_OK and Z_NEED_DICT, I suggest replacing:
    this_off += this_len - strm->avail_in;
    (*env)->SetIntField(env, this, offID, this_off);
with:
    (*env)->SetIntField(env, this, offID, this_len - strm->avail_in);
as this_off is not further used in the function. 

Thanks!
         Dave

--- On Fri, 4/1/11, Xueming Shen <xueming.shen at oracle.com> wrote:

> From: Xueming Shen <xueming.shen at oracle.com>
> Subject: Review Request for 6751338: ZIP inflater/deflater performance
> To: "Dave Bristor" <bristor at yahoo.com>, "BATEMAN,ALAN" <alan.bateman at oracle.com>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Date: Friday, April 1, 2011, 4:04 PM
> Dave, Alan,
> 
> Here is the final webrev based on Dave's patch and the
> jdk1.5 code that does not
> have the change for 6206933. JPRT job result suggests no
> new testing failure and
> my "non-scientific" benchmark test (to use
> GZIPOu/InputStream to compress/
> decompress the rt.jar) does show relative performance gain.
> Will try to run more
> tests the weekend, but here is the webrev.
> 
> http://cr.openjdk.java.net/~sherman/6751338/webrev/
> 
> Background Info:
> 
> This fix is basically to back out the fix for #6206933 we
> made back to jdk5, which
> is to use malloc+GetByteArrayuRegion to replace the
> original GetPrimitiveArrayCritical/
> ReleasePrimitiveArrayCritical() pair when access the java
> byte array at native code
> Inflater/Deflater.c, to mainly workaround the
> GC/Critical... issue discussed in
> #6186200.
> 
> The change for #6206933 itself has triggered lots of
> performance issues
> since its integration, some fixed, some still outstanding.
> The GC rfe#6186200 has
> been fixed long time ago, after couple weeks of
> discussion/debating, we all agreed
> that it's the time to back out#6206933.
> 
> -Sherman
> 
>