RFR: 8098581 SecureRandom.nextBytes() hurts performance with small size requests

[ecki at zusammenkunft.net]

Hello,

That buffer sizing/expiring is a bit strange (the dynmic version stranger than the old one), I do not understand the need for it. The requests should not varry so much in size. Just reading something like 64bytes at a time (possibly nonblocking) should be fine. No need to look at the age of that buffer...

A cool implementation would be an actual single periodic refresher thread which does not block reading (until buffer is empty)... but that might change the semantic a bit (and could be better used with the new DRNG api for block-free reseeding).

Bernd

-- 
http://bernd.eckenfels.net

-----Original Message-----
From: Mike StJohns <mstjohns at comcast.net>
To: security-dev at openjdk.java.net
Sent: Do., 03 Dez. 2015 21:22
Subject: Re: RFR: 8098581 SecureRandom.nextBytes() hurts performance with small size requests

The more I look at this the less I'm comfortable with the change to 
non-synchronized.

In general, there shouldn't be a need for a large amount of random data, 
especially not between multiple threads, so I'm not too sure the 
performance argument holds.

What I do worry about is a provider that's not particularly smart about 
how it does things offering up data and not updating internal pointers 
in a synchronized manner resulting in the possibility of the same 
"random" data being served to multiple requestors.   I think placing the 
synchronization at the glue class may be a better choice than depending 
on each and every provider doing the right thing.

Just a thought - Mike



On 12/3/2015 1:38 PM, Seán Coffey wrote:
> Hey Tony,
>
> looks like a fix that could come back to jdk8u-dev at some stage. 
> Thanks for taking this one on.
>
> Adding SecureRandom to the sunpkcs11-solaris.cfg file is a nice move. 
> Would you agree that JDK-8058455 could be closed out as a duplicate ?
>
> Regards,
> Sean.
>
> On 01/12/2015 23:44, Anthony Scarpino wrote:
>> Hi all,
>>
>> I'd like a review of this change.  It improves nextBytes() 
>> performance by allowing the random buffer to grow and shrink as 
>> random data is needed and remove the high level synchronization. Also 
>> disable SecureRandom for Solaris PKCS11 as it's not as fast as native.
>>
>> http://cr.openjdk.java.net/~ascarpino/8098581/webrev/
>>
>> Thanks
>>
>> Tony
>
>