On Mon, 14 Nov 2022 16:37:41 GMT, Xubo Zhang <d...@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 <turban...@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