RFR: 8296746: NativePRNG SecureRandom doesn't scale with threads [v2]

Wed Nov 16 21:02:48 UTC 2022

On Mon, 14 Nov 2022 16:37:41 GMT, Xubo Zhang <duke at openjdk.org> wrote:

>> NativePRNG SecureRandom doesn’t scale with number of threads. The performance starts dropping as we increase the number of threads. Even going from 1 thread to 2 threads shows significant drop. The bottleneck is the singleton RandomIO instance. Making the RandomIO ThreadLocal helps in removing this bottleneck.
>> 
>> Here are the jmh  thrpt ops/s data for SecureRandomBench.nextBytes, algorithm=NativePRNGNonBlocking, datasize=64, notshared:
>> 
>> <html xmlns:v="urn:schemas-microsoft-com:vml"
>> xmlns:o="urn:schemas-microsoft-com:office:office"
>> xmlns:x="urn:schemas-microsoft-com:office:excel"
>> xmlns="http://www.w3.org/TR/REC-html40">
>> 
>> <head>
>> 
>> <meta name=ProgId content=Excel.Sheet>
>> <meta name=Generator content="Microsoft Excel 15">
>> <link id=Main-File rel=Main-File
>> href="file:///C:/Users/xbzhang/AppData/Local/Temp/msohtmlclip1/01/clip.htm">
>> <link rel=File-List
>> href="file:///C:/Users/xbzhang/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml">
>> 
>> 
>> 
>> </head>
>> 
>> <body link="#0563C1" vlink="#954F72">
>> 
>> 
>> 
>> #threads | singleton RandomIO | ThreadLocal RandomIO
>> -- | -- | --
>> 1 | 1269133 | 1279088
>> 2 | 862923 | 1362066
>> 3 | 819734 | 1630522
>> 4 | 809128 | 1614500
>> 5 | 821514 | 1594965
>> 6 | 808795 | 1545045
>> 7 | 783050 | 1499388
>> 8 | 787236 | 1470004
>> 
>> 
>> 
>> </body>
>> 
>> </html>
>
> Xubo Zhang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update src/java.base/unix/classes/sun/security/provider/NativePRNG.java
>   
>   Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>

Any ideas why it is not scaling linearly with number of threads? One would think that random numbers are the 'definition' of data-independent and should scale linearly with number of CPUs

src/java.base/unix/classes/sun/security/provider/NativePRNG.java line 93:

> 91: 
> 92:     // ThreadLocal instance or null if not available
> 93:     private static final ThreadLocal<RandomIO> INSTANCE = initIO(Variant.MIXED);

Should this be a new Variant instead? (i.e. keep the old behavior of the other enum values)

You would need to create a new nested class (perhaps only copy-and-modify the non-blocking one?)

src/java.base/unix/classes/sun/security/provider/NativePRNG.java line 204:

> 202:     // return whether the NativePRNG is available
> 203:     static boolean isAvailable() {
> 204:         return INSTANCE.get() != null;

I think this would be cleaner if it were `return INSTANCE() != null;`

i.e. replace the field with a getter. Existing variants return just the field, while thread-local calls `get()`.

-------------

PR: https://git.openjdk.org/jdk/pull/11069