8098581: SecureRandom.nextBytes() hurts performance with small size requests
authorascarpino
Mon, 08 Feb 2016 13:09:16 -0800
changeset 35718 e6b4f7af3fca
parent 35717 6d49715a72e5
child 35719 16e7f825b13a
8098581: SecureRandom.nextBytes() hurts performance with small size requests Reviewed-by: valeriep
jdk/src/java.base/share/classes/java/security/SecureRandom.java
jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java
jdk/src/jdk.crypto.pkcs11/solaris/conf/security/sunpkcs11-solaris.cfg
jdk/test/java/security/SecureRandom/DefaultProvider.java
--- a/jdk/src/java.base/share/classes/java/security/SecureRandom.java	Mon Feb 08 10:46:42 2016 -0800
+++ b/jdk/src/java.base/share/classes/java/security/SecureRandom.java	Mon Feb 08 13:09:16 2016 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -472,7 +472,7 @@
      * @param bytes the array to be filled in with random bytes.
      */
     @Override
-    public synchronized void nextBytes(byte[] bytes) {
+    public void nextBytes(byte[] bytes) {
         secureRandomSpi.engineNextBytes(bytes);
     }
 
--- a/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java	Mon Feb 08 10:46:42 2016 -0800
+++ b/jdk/src/java.base/unix/classes/sun/security/provider/NativePRNG.java	Mon Feb 08 13:09:16 2016 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,6 +28,8 @@
 import java.io.*;
 import java.net.*;
 import java.security.*;
+import java.util.Arrays;
+
 import sun.security.util.Debug;
 
 /**
@@ -334,7 +336,9 @@
         private static final long MAX_BUFFER_TIME = 100;
 
         // size of the "next" buffer
-        private static final int BUFFER_SIZE = 32;
+        private static final int MAX_BUFFER_SIZE = 65536;
+        private static final int MIN_BUFFER_SIZE = 32;
+        private int bufferSize = 256;
 
         // Holder for the seedFile.  Used if we ever add seed material.
         File seedFile;
@@ -351,7 +355,7 @@
         private volatile sun.security.provider.SecureRandom mixRandom;
 
         // buffer for next bits
-        private final byte[] nextBuffer;
+        private byte[] nextBuffer;
 
         // number of bytes left in nextBuffer
         private int buffered;
@@ -359,6 +363,16 @@
         // time we read the data into the nextBuffer
         private long lastRead;
 
+        // Count for the number of buffer size changes requests
+        // Positive value in increase size, negative to lower it.
+        private int change_buffer = 0;
+
+        // Request limit to trigger an increase in nextBuffer size
+        private static final int REQ_LIMIT_INC = 1000;
+
+        // Request limit to trigger a decrease in nextBuffer size
+        private static final int REQ_LIMIT_DEC = -100;
+
         // mutex lock for nextBytes()
         private final Object LOCK_GET_BYTES = new Object();
 
@@ -373,7 +387,7 @@
             this.seedFile = seedFile;
             seedIn = FileInputStreamPool.getInputStream(seedFile);
             nextIn = FileInputStreamPool.getInputStream(nextFile);
-            nextBuffer = new byte[BUFFER_SIZE];
+            nextBuffer = new byte[bufferSize];
         }
 
         // get the SHA1PRNG for mixing
@@ -466,9 +480,47 @@
         // if not, read new bytes
         private void ensureBufferValid() throws IOException {
             long time = System.currentTimeMillis();
-            if ((buffered > 0) && (time - lastRead < MAX_BUFFER_TIME)) {
-                return;
+            int new_buffer_size = 0;
+
+            // Check if buffer has bytes available that are not too old
+            if (buffered > 0) {
+                if (time - lastRead < MAX_BUFFER_TIME) {
+                    return;
+                } else {
+                    // byte is old, so subtract from counter to shrink buffer
+                    change_buffer--;
+                }
+            } else {
+                // No bytes available, so add to count to increase buffer
+                change_buffer++;
+            }
+
+            // If counter has it a limit, increase or decrease size
+            if (change_buffer > REQ_LIMIT_INC) {
+                new_buffer_size = nextBuffer.length * 2;
+            } else if (change_buffer < REQ_LIMIT_DEC) {
+                new_buffer_size = nextBuffer.length / 2;
             }
+
+            // If buffer size is to be changed, replace nextBuffer.
+            if (new_buffer_size > 0) {
+                if (new_buffer_size <= MAX_BUFFER_SIZE &&
+                        new_buffer_size >= MIN_BUFFER_SIZE) {
+                    nextBuffer = new byte[new_buffer_size];
+                    if (debug != null) {
+                        debug.println("Buffer size changed to " +
+                                new_buffer_size);
+                    }
+                } else {
+                    if (debug != null) {
+                        debug.println("Buffer reached limit: " +
+                                nextBuffer.length);
+                    }
+                }
+                change_buffer = 0;
+            }
+
+            // Load fresh random bytes into nextBuffer
             lastRead = time;
             readFully(nextIn, nextBuffer);
             buffered = nextBuffer.length;
@@ -478,24 +530,40 @@
         // read from "next" and XOR with bytes generated by the
         // mixing SHA1PRNG
         private void implNextBytes(byte[] data) {
-            synchronized (LOCK_GET_BYTES) {
                 try {
                     getMixRandom().engineNextBytes(data);
-                    int len = data.length;
+                    int data_len = data.length;
                     int ofs = 0;
-                    while (len > 0) {
-                        ensureBufferValid();
-                        int bufferOfs = nextBuffer.length - buffered;
-                        while ((len > 0) && (buffered > 0)) {
-                            data[ofs++] ^= nextBuffer[bufferOfs++];
-                            len--;
-                            buffered--;
+                    int len;
+                    int buf_pos;
+                    int localofs;
+                    byte[] localBuffer;
+
+                    while (data_len > 0) {
+                        synchronized (LOCK_GET_BYTES) {
+                            ensureBufferValid();
+                            buf_pos = nextBuffer.length - buffered;
+                            if (data_len > buffered) {
+                                len = buffered;
+                                buffered = 0;
+                            } else {
+                                len = data_len;
+                                buffered -= len;
+                            }
+                            localBuffer = Arrays.copyOfRange(nextBuffer, buf_pos,
+                                    buf_pos + len);
                         }
+                        localofs = 0;
+                        while (len > localofs) {
+                            data[ofs] ^= localBuffer[localofs];
+                            ofs++;
+                            localofs++;
+                        }
+                    data_len -= len;
                     }
-                } catch (IOException e) {
+                } catch (IOException e){
                     throw new ProviderException("nextBytes() failed", e);
                 }
-            }
+        }
         }
-    }
 }
--- a/jdk/src/jdk.crypto.pkcs11/solaris/conf/security/sunpkcs11-solaris.cfg	Mon Feb 08 10:46:42 2016 -0800
+++ b/jdk/src/jdk.crypto.pkcs11/solaris/conf/security/sunpkcs11-solaris.cfg	Mon Feb 08 13:09:16 2016 -0800
@@ -18,5 +18,6 @@
 
 disabledMechanisms = {
   CKM_DSA_KEY_PAIR_GEN
+  SecureRandom
 }
 
--- a/jdk/test/java/security/SecureRandom/DefaultProvider.java	Mon Feb 08 10:46:42 2016 -0800
+++ b/jdk/test/java/security/SecureRandom/DefaultProvider.java	Mon Feb 08 13:09:16 2016 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -43,12 +43,7 @@
         out.println("TEST: Default provider with constructor");
         SecureRandom secureRandom = new SecureRandom();
         String provider = secureRandom.getProvider().getName();
-        if (OS_NAME.startsWith(SUNOS)) {
-            if (!provider.startsWith("SunPKCS11-")) {
-                throw new RuntimeException("Unexpected provider name: "
-                        + provider);
-            }
-        } else if (!provider.equals("SUN")) {
+        if (!provider.equals("SUN")) {
             throw new RuntimeException("Unexpected provider name: "
                     + provider);
         }
@@ -77,16 +72,6 @@
         instance = SecureRandom.getInstance(algorithm);
         assertInstance(instance, algorithm, provider);
         out.println("Passed.");
-
-        if (OS_NAME.startsWith(SUNOS)) {
-            out.println(
-                    "TEST: PKCS11 is supported on Solaris by SunPKCS11 provider");
-            algorithm = "PKCS11";
-            provider = "SunPKCS11-Solaris";
-            instance = SecureRandom.getInstance(algorithm);
-            assertInstance(instance, algorithm, provider);
-            out.println("Passed.");
-        }
     }
 
     private static void assertInstance(SecureRandom instance,