7157903: JSSE client sockets are very slow
authorwetmore
Wed, 11 Apr 2012 17:12:35 -0700
changeset 12428 e9feb65d37fa
parent 12427 116544b5a04c
child 12429 91210fbc1747
7157903: JSSE client sockets are very slow Reviewed-by: xuelei
jdk/src/share/classes/sun/security/ssl/AppOutputStream.java
jdk/src/share/classes/sun/security/ssl/EngineOutputRecord.java
jdk/src/share/classes/sun/security/ssl/OutputRecord.java
jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java
--- a/jdk/src/share/classes/sun/security/ssl/AppOutputStream.java	Wed Apr 11 07:26:35 2012 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/AppOutputStream.java	Wed Apr 11 17:12:35 2012 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2012, 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
@@ -91,9 +91,20 @@
         // however they like; if we buffered here, they couldn't.
         try {
             do {
+                boolean holdRecord = false;
                 int howmuch;
                 if (isFirstRecordOfThePayload && c.needToSplitPayload()) {
                     howmuch = Math.min(0x01, r.availableDataBytes());
+                     /*
+                      * Nagle's algorithm (TCP_NODELAY) was coming into
+                      * play here when writing short (split) packets.
+                      * Signal to the OutputRecord code to internally
+                      * buffer this small packet until the next outbound
+                      * packet (of any type) is written.
+                      */
+                     if ((len != 1) && (howmuch == 1)) {
+                         holdRecord = true;
+                     }
                 } else {
                     howmuch = Math.min(len, r.availableDataBytes());
                 }
@@ -108,7 +119,7 @@
                     off += howmuch;
                     len -= howmuch;
                 }
-                c.writeRecord(r);
+                c.writeRecord(r, holdRecord);
                 c.checkWrite();
             } while (len > 0);
         } catch (Exception e) {
--- a/jdk/src/share/classes/sun/security/ssl/EngineOutputRecord.java	Wed Apr 11 07:26:35 2012 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/EngineOutputRecord.java	Wed Apr 11 17:12:35 2012 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, 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
@@ -155,8 +155,9 @@
      * data to be generated/output before the exception is ever
      * generated.
      */
-    void writeBuffer(OutputStream s, byte [] buf, int off, int len)
-            throws IOException {
+    @Override
+    void writeBuffer(OutputStream s, byte [] buf, int off, int len,
+            int debugOffset) throws IOException {
         /*
          * Copy data out of buffer, it's ready to go.
          */
@@ -196,7 +197,8 @@
             // compress();              // eventually
             addMAC(writeMAC);
             encrypt(writeCipher);
-            write((OutputStream)null);  // send down for processing
+            write((OutputStream)null, false,  // send down for processing
+                (ByteArrayOutputStream)null);
         }
         return;
     }
--- a/jdk/src/share/classes/sun/security/ssl/OutputRecord.java	Wed Apr 11 07:26:35 2012 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/OutputRecord.java	Wed Apr 11 17:12:35 2012 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2012, 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,7 @@
 
 import java.io.*;
 import java.nio.*;
+import java.util.Arrays;
 
 import javax.net.ssl.SSLException;
 import sun.misc.HexDumpEncoder;
@@ -227,6 +228,24 @@
     }
 
     /*
+     * Increases the capacity if necessary to ensure that it can hold
+     * at least the number of elements specified by the minimum
+     * capacity argument.
+     *
+     * Note that the increased capacity is only can be used for held
+     * record buffer. Please DO NOT update the availableDataBytes()
+     * according to the expended buffer capacity.
+     *
+     * @see availableDataBytes()
+     */
+    private void ensureCapacity(int minCapacity) {
+        // overflow-conscious code
+        if (minCapacity > buf.length) {
+            buf = Arrays.copyOf(buf, minCapacity);
+        }
+    }
+
+    /*
      * Return the type of SSL record that's buffered here.
      */
     final byte contentType() {
@@ -243,7 +262,9 @@
      * that synchronization be done elsewhere.  Also, this does its work
      * in a single low level write, for efficiency.
      */
-    void write(OutputStream s) throws IOException {
+    void write(OutputStream s, boolean holdRecord,
+            ByteArrayOutputStream heldRecordBuffer) throws IOException {
+
         /*
          * Don't emit content-free records.  (Even change cipher spec
          * messages have a byte of data!)
@@ -300,7 +321,49 @@
         }
         firstMessage = false;
 
-        writeBuffer(s, buf, 0, count);
+        /*
+         * The upper levels may want us to delay sending this packet so
+         * multiple TLS Records can be sent in one (or more) TCP packets.
+         * If so, add this packet to the heldRecordBuffer.
+         *
+         * NOTE:  all writes have been synchronized by upper levels.
+         */
+        int debugOffset = 0;
+        if (holdRecord) {
+            /*
+             * If holdRecord is true, we must have a heldRecordBuffer.
+             *
+             * Don't worry about the override of writeBuffer(), because
+             * when holdRecord is true, the implementation in this class
+             * will be used.
+             */
+            writeBuffer(heldRecordBuffer, buf, 0, count, debugOffset);
+        } else {
+            // It's time to send, do we have buffered data?
+            // May or may not have a heldRecordBuffer.
+            if (heldRecordBuffer != null && heldRecordBuffer.size() > 0) {
+                int heldLen = heldRecordBuffer.size();
+
+                // Ensure the capacity of this buffer.
+                ensureCapacity(count + heldLen);
+
+                // Slide everything in the buffer to the right.
+                System.arraycopy(buf, 0, buf, heldLen, count);
+
+                // Prepend the held record to the buffer.
+                System.arraycopy(
+                    heldRecordBuffer.toByteArray(), 0, buf, 0, heldLen);
+                count += heldLen;
+
+                // Clear the held buffer.
+                heldRecordBuffer.reset();
+
+                // The held buffer has been dumped, set the debug dump offset.
+                debugOffset = heldLen;
+            }
+            writeBuffer(s, buf, 0, count, debugOffset);
+        }
+
         reset();
     }
 
@@ -309,15 +372,17 @@
      * we'll override this method and let it take the appropriate
      * action.
      */
-    void writeBuffer(OutputStream s, byte [] buf, int off, int len)
-            throws IOException {
+    void writeBuffer(OutputStream s, byte [] buf, int off, int len,
+            int debugOffset) throws IOException {
         s.write(buf, off, len);
         s.flush();
 
+        // Output only the record from the specified debug offset.
         if (debug != null && Debug.isOn("packet")) {
             try {
                 HexDumpEncoder hd = new HexDumpEncoder();
-                ByteBuffer bb = ByteBuffer.wrap(buf, off, len);
+                ByteBuffer bb = ByteBuffer.wrap(
+                        buf, off + debugOffset, len - debugOffset);
 
                 System.out.println("[Raw write]: length = " +
                     bb.remaining());
--- a/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java	Wed Apr 11 07:26:35 2012 -0700
+++ b/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java	Wed Apr 11 17:12:35 2012 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2012, 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
@@ -374,6 +374,12 @@
      */
     private boolean isFirstAppOutputRecord = true;
 
+    /*
+     * If AppOutputStream needs to delay writes of small packets, we
+     * will use this to store the data until we actually do the write.
+     */
+    private ByteArrayOutputStream heldRecordBuffer = null;
+
     //
     // CONSTRUCTORS AND INITIALIZATION CODE
     //
@@ -654,13 +660,24 @@
     //
 
     /*
+     * AppOutputStream calls may need to buffer multiple outbound
+     * application packets.
+     *
+     * All other writeRecord() calls will not buffer, so do not hold
+     * these records.
+     */
+    void writeRecord(OutputRecord r) throws IOException {
+        writeRecord(r, false);
+    }
+
+    /*
      * Record Output. Application data can't be sent until the first
      * handshake establishes a session.
      *
      * NOTE:  we let empty records be written as a hook to force some
      * TCP-level activity, notably handshaking, to occur.
      */
-    void writeRecord(OutputRecord r) throws IOException {
+    void writeRecord(OutputRecord r, boolean holdRecord) throws IOException {
         /*
          * The loop is in case of HANDSHAKE --> ERROR transitions, etc
          */
@@ -731,7 +748,7 @@
                 try {
                     if (writeLock.tryLock(getSoLinger(), TimeUnit.SECONDS)) {
                         try {
-                            writeRecordInternal(r);
+                            writeRecordInternal(r, holdRecord);
                         } finally {
                             writeLock.unlock();
                         }
@@ -779,7 +796,7 @@
             } else {
                 writeLock.lock();
                 try {
-                    writeRecordInternal(r);
+                    writeRecordInternal(r, holdRecord);
                 } finally {
                     writeLock.unlock();
                 }
@@ -787,11 +804,29 @@
         }
     }
 
-    private void writeRecordInternal(OutputRecord r) throws IOException {
+    private void writeRecordInternal(OutputRecord r,
+            boolean holdRecord) throws IOException {
+
         // r.compress(c);
         r.addMAC(writeMAC);
         r.encrypt(writeCipher);
-        r.write(sockOutput);
+
+        if (holdRecord) {
+            // If we were requested to delay the record due to possibility
+            // of Nagle's being active when finally got to writing, and
+            // it's actually not, we don't really need to delay it.
+            if (getTcpNoDelay()) {
+                holdRecord = false;
+            } else {
+                // We need to hold the record, so let's provide
+                // a per-socket place to do it.
+                if (heldRecordBuffer == null) {
+                    // Likely only need 37 bytes.
+                    heldRecordBuffer = new ByteArrayOutputStream(40);
+                }
+            }
+        }
+        r.write(sockOutput, holdRecord, heldRecordBuffer);
 
         /*
          * Check the sequence number state