Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-03 Thread Seán Coffey
Thanks for the review Tony. Yes - no problems with tests. Copyright updated and changeset pushed. regards, Sean. On 03/08/2018 05:23, Anthony Scarpino wrote: Looks good. Only comment is update the copyright year. No need to regenerate a webrev. Assuming it passes all the tests I say it’s rea

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-02 Thread Anthony Scarpino
Looks good. Only comment is update the copyright year. No need to regenerate a webrev. Assuming it passes all the tests I say it’s ready to push. Tony > On Aug 2, 2018, at 5:15 AM, Seán Coffey wrote: > > Thanks Tony. I was asked off thread to add some comments to help code > maintenance also

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-02 Thread Seán Coffey
Thanks Tony. I was asked off thread to add some comments to help code maintenance also. webrev updated. Hope we're nearly ready to push now. http://cr.openjdk.java.net/~coffeys/webrev.8207775.v4/webrev/ regards, Sean. On 01/08/2018 18:41, Anthony Scarpino wrote: That looks fine to me. Tony

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-01 Thread Anthony Scarpino
That looks fine to me. Tony On 08/01/2018 08:39 AM, Seán Coffey wrote: Thanks again for review Tony. I think you raise a good point and should give some performance gain. for line 676: maybe this : byte[] copy = Arrays.copyOf(output, len); if (decrypting)

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-01 Thread Seán Coffey
Thanks again for review Tony. I think you raise a good point and should give some performance gain. for line 676: maybe this : byte[] copy = Arrays.copyOf(output, len); if (decrypting) { Arrays.fill(output, (byte) 0x00); }

Re: RFR: 8207775: Better management of CipherCore buffers

2018-08-01 Thread Anthony Scarpino
My only comment is if it makes sense to have the change at 676 to also only null out on decrypt? Otherwise I'm fine with the changes Tony On 07/31/2018 02:04 AM, Seán Coffey wrote: Thanks for review Tony. Comments inline.. On 27/07/18 21:02, Anthony Scarpino wrote: If we are going to ad

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-31 Thread Seán Coffey
Thanks for review Tony. Comments inline.. On 27/07/18 21:02, Anthony Scarpino wrote: If we are going to add more, here are two more ton consider: - It looks like there is another Arrays.copyOf() on doFinal() line 851 Good point. - doFinal() at line 897 there might be something that should be

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-27 Thread Anthony Scarpino
If we are going to add more, here are two more ton consider: - It looks like there is another Arrays.copyOf() on doFinal() line 851 - doFinal() at line 897 there might be something that should be done with 'buffer'. In particular as a result of line 963's arraycopy(). Tony On 07/27/2018 08:

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-27 Thread Seán Coffey
Thanks Tony. If it's alright with you, I'd like to make one more edit for this change. http://cr.openjdk.java.net/~coffeys/webrev.8207775.v2/webrev/ There's a condition where we can null out an array early if we're returning a copy. See lines 671-683 Regards, Sean. On 26/07/18 17:42, Anthon

Re: RFR: 8207775: Better management of CipherCore buffers

2018-07-26 Thread Anthony Scarpino
On 07/26/2018 07:36 AM, Seán Coffey wrote: https://bugs.openjdk.java.net/browse/JDK-8207775 Simple enough fix to null out some internal buffers once they're no longer required. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8207775/webrev/ regards, Sean. that looks fine.. Tony

RFR: 8207775: Better management of CipherCore buffers

2018-07-26 Thread Seán Coffey
https://bugs.openjdk.java.net/browse/JDK-8207775 Simple enough fix to null out some internal buffers once they're no longer required. webrev : http://cr.openjdk.java.net/~coffeys/webrev.8207775/webrev/ regards, Sean.