@Alan: Great idea about looking at runs with +PrintGCDetails and
+PrintJNIGCStalls! Wish I had thought of that. About @ignore: it seems fine
remove that, though if there are performance degradations the test won't catch
them of course. Renaming the test seems fine.
@Sherman: updating this_off is fine as "documentation".
Dave
--- On Mon, 4/4/11, Alan Bateman <[email protected]> wrote:
> From: Alan Bateman <[email protected]>
> Subject: Re: Review Request for 6751338: ZIP inflater/deflater performance
> To: "Xueming Shen" <[email protected]>
> Cc: "Dave Bristor" <[email protected]>, "core-libs-dev"
> <[email protected]>
> Date: Monday, April 4, 2011, 2:09 AM
> Xueming Shen wrote:
> > 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/
> I went through the webrev and also checked the old
> (pre-OpenJDK) code from before the changes for 6206933. The
> changes look okay to me. When testing with HotSpot then
> running with +PrintGCDetails and +PrintJNIGCStalls may be
> useful.
>
> One comment on the FlaterCriticalArray.java test is that it
> might be better to push this without the jtreg tags as the
> @ignore will cause it to be reported by jtreg as an "error".
> On the naming then maybe InflateDelatePerf.java might be
> better.
>
> -Alan
>