On Wed, Apr 22, 2020 at 10:45 PM Claes Redestad
wrote:
>
>
>
> On 2020-04-22 22:08, Volker Simonis wrote:
> > http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848.02/
> >
> > Notice that this new version only changes the microbenchmark, all the
> > other files are untouched.
> >
> > As everybo
On 16/04/2020 07:48, Thomas Stüfe wrote:
As for increasing the buffer size, it makes sense but IMHO needs a CSR and
a release note.
Yes, a release note would be appropriate here. Lance is going to dig
into the history to try to find the rational for the original code in
case there is something
ms/op
>> InflaterOutputStreamWrite.write 8 32768 avgt 3 1.979 ±
>> 0.119 ms/op
>> InflaterOutputStreamWrite.write8 65536 avgt3 1.986 ±
>> 0.155 ms/op
>>
>> >
>> > Thanks,
>> > Laurent
>> >
>> >
On Thu, Apr 23, 2020 at 1:22 PM Lance Andersen
wrote:
> Hi Volker
>
> On Apr 22, 2020, at 4:08 PM, Volker Simonis
> wrote:
>
> On Tue, Apr 21, 2020 at 5:23 PM Lance Andersen
> wrote:
>
>
> Hi Volker,
>
> I think overall this looks OK. I went through the older SCCS histories to
> see if I could
> -Original Message-
> > From: core-libs-dev On Behalf
> > Of Volker Simonis
> > Sent: Mittwoch, 22. April 2020 22:09
> > To: Lance Andersen
> > Cc: Java Core Libs ; Vyom Tewari
> >
> > Subject: Re: RFR(S): 8242848: Improve performance of
and I like it. +1
> >>
> >> One little style nit caught my eye, which you could correct before
> pushing: The style of the if/else blocks in
> test/jdk/java/util/zip/DeflateIn_InflateOut.java in lines 48/49 and 59/60
> does not match the other if/else blocks in the fil
Hi Volker
> On Apr 22, 2020, at 4:08 PM, Volker Simonis wrote:
>
> On Tue, Apr 21, 2020 at 5:23 PM Lance Andersen
> wrote:
>>
>> Hi Volker,
>>
>> I think overall this looks OK. I went through the older SCCS histories to
>> see if I could figure out why they were using 512 for the input leng
inal Message-
>> > From: core-libs-dev On Behalf
>> > Of Volker Simonis
>> > Sent: Mittwoch, 22. April 2020 22:09
>> > To: Lance Andersen
>> > Cc: Java Core Libs ; Vyom Tewari
>> >
>> > Subject: Re: RFR(S): 8242848: Improve perf
the line of the closing bracket before...
>
> Thanks
> Christoph
>
>
> > -Original Message-
> > From: core-libs-dev On Behalf
> > Of Volker Simonis
> > Sent: Mittwoch, 22. April 2020 22:09
> > To: Lance Andersen
> > Cc: Java Co
> Cc: Java Core Libs ; Vyom Tewari
>
> Subject: Re: RFR(S): 8242848: Improve performance of
> InflaterOutputStream.write()
>
> On Tue, Apr 21, 2020 at 5:23 PM Lance Andersen
> wrote:
> >
> > Hi Volker,
> >
> > I think overall this looks OK. I went through
Typo in the benchmark javadoc: “avarage” instead of “average"
> On Apr 22, 2020, at 4:47 PM, Claes Redestad wrote:
>
>
>
> On 2020-04-22 22:08, Volker Simonis wrote:
>> http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848.02/
>> Notice that this new version only changes the microbenchmark,
On 2020-04-22 22:08, Volker Simonis wrote:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848.02/
Notice that this new version only changes the microbenchmark, all the
other files are untouched.
As everybody seemed to be happy with the change itself and the
regression test, I'm now wai
On Tue, Apr 21, 2020 at 5:23 PM Lance Andersen
wrote:
>
> Hi Volker,
>
> I think overall this looks OK. I went through the older SCCS histories to
> see if I could figure out why they were using 512 for the input length but
> could not find anything that might shed some light for me.
>
Hi Lanc
On Tue, Apr 21, 2020 at 4:38 PM Thomas Stüfe wrote:
>
> Hi Volker,
>
> looks good now. Only a nit, really, but since this is pretty much binary now
> (we either have the whole input or have it given to the inflater), setting
> len=0 seems weird, maybe add a boolean like "inputFedToInflater"? But
Hi Volker,
I think overall this looks OK. I went through the older SCCS histories to see
if I could figure out why they were using 512 for the input length but could
not find anything that might shed some light for me.
I am not sure you can guarantee that src.zip exists but others might be abl
Hi Volker,
looks good now. Only a nit, really, but since this is pretty much binary
now (we either have the whole input or have it given to the inflater),
setting len=0 seems weird, maybe add a boolean like "inputFedToInflater"?
But I leave it up to you, this is just idle nitpicking. I am fine wit
On Mon, Apr 20, 2020 at 5:41 AM Vyom Tiwari wrote:
>
> Hi Volker,
>
> Latest changes looks good to me. I notices the copyright in new test
> Streams.java is "
>
> Copyright (c) 2020, Amazon and/or its affiliates." instead is regular Oracle
> copyright.
>
Hi Vyom,
thanks a lot for your review.
Hi Volker,
Latest changes looks good to me. I notices the copyright in new test
Streams.java is "
Copyright (c) 2020, Amazon and/or its affiliates." instead is regular
Oracle copyright.
Thanks,
Vyom
On Fri, Apr 17, 2020 at 4:23 PM Volker Simonis
wrote:
> Thanks everybody for looking at this c
Thanks everybody for looking at this change!
Please find an updated webrev (with a new test and micro-benchmark)
and my answers to your comments below:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848.01/
On Thu, Apr 16, 2020 at 6:40 AM Vyom Tiwari wrote:
> Thanks for doing this, i thin
Hi Volker,
On 2020-04-15 19:48, Volker Simonis wrote:
While doing this change, I've also realized that all the streams in
java.util.zip (i.e. DeflaterInputStream, GZIPInputStream,
GZIPOutputStream, InflaterInputStream, DeflaterOutputStream) use an
internal byte buffer of 512 bytes by default. Lo
As for increasing the buffer size, it makes sense but IMHO needs a CSR and
a release note.
Cheers, Thomas
On Thu, Apr 16, 2020 at 8:21 AM Thomas Stüfe
wrote:
> Hi Volker,
>
> 252 // Check the decompressor
> 253 if (inf.finished() || (len == 0)/* no more input *
Hi Volker,
252 // Check the decompressor
253 if (inf.finished() || (len == 0)/* no more input */) {
254 break;
255 }
Not sure but I think this is wrong because now you bypass the followup
handling of inf.needsDirectory.
Infl
Hi Volker,
Thanks for doing this, i think the below code change is not required .
Please do let me know if i am not reading it correctly.
if (inf.finished() || (len == 0)/* no more input */) {
If you check the native code "inf.finished() will be true only of the
corresponding native call inflat
23 matches
Mail list logo