From dd7a928d3af171dc6360d7b3e6732d07c4f67789 Mon Sep 17 00:00:00 2001 From: wetmore Date: Wed, 11 Apr 2012 17:12:35 -0700 Subject: [PATCH] 7157903: JSSE client sockets are very slow Reviewed-by: xuelei --- .../sun/security/ssl/AppOutputStream.java | 15 +++- .../sun/security/ssl/EngineOutputRecord.java | 10 ++- .../sun/security/ssl/OutputRecord.java | 77 +++++++++++++++++-- .../sun/security/ssl/SSLSocketImpl.java | 47 +++++++++-- 4 files changed, 131 insertions(+), 18 deletions(-) diff --git a/src/share/classes/sun/security/ssl/AppOutputStream.java b/src/share/classes/sun/security/ssl/AppOutputStream.java index 6be00b8e2..f9f6e9e8a 100644 --- a/src/share/classes/sun/security/ssl/AppOutputStream.java +++ b/src/share/classes/sun/security/ssl/AppOutputStream.java @@ -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 @@ class AppOutputStream extends OutputStream { // 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 @@ class AppOutputStream extends OutputStream { off += howmuch; len -= howmuch; } - c.writeRecord(r); + c.writeRecord(r, holdRecord); c.checkWrite(); } while (len > 0); } catch (Exception e) { diff --git a/src/share/classes/sun/security/ssl/EngineOutputRecord.java b/src/share/classes/sun/security/ssl/EngineOutputRecord.java index 707fd04f8..e5eb7f759 100644 --- a/src/share/classes/sun/security/ssl/EngineOutputRecord.java +++ b/src/share/classes/sun/security/ssl/EngineOutputRecord.java @@ -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 @@ final class EngineOutputRecord extends OutputRecord { * 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 @@ final class EngineOutputRecord extends OutputRecord { // compress(); // eventually addMAC(writeMAC); encrypt(writeCipher); - write((OutputStream)null); // send down for processing + write((OutputStream)null, false, // send down for processing + (ByteArrayOutputStream)null); } return; } diff --git a/src/share/classes/sun/security/ssl/OutputRecord.java b/src/share/classes/sun/security/ssl/OutputRecord.java index e5c7a6def..ca3cf7bb9 100644 --- a/src/share/classes/sun/security/ssl/OutputRecord.java +++ b/src/share/classes/sun/security/ssl/OutputRecord.java @@ -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 @@ package sun.security.ssl; import java.io.*; import java.nio.*; +import java.util.Arrays; import javax.net.ssl.SSLException; import sun.misc.HexDumpEncoder; @@ -226,6 +227,24 @@ class OutputRecord extends ByteArrayOutputStream implements Record { return maxDataSize - dataSize; } + /* + * 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. */ @@ -243,7 +262,9 @@ class OutputRecord extends ByteArrayOutputStream implements Record { * 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 @@ class OutputRecord extends ByteArrayOutputStream implements Record { } 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 @@ class OutputRecord extends ByteArrayOutputStream implements Record { * 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()); diff --git a/src/share/classes/sun/security/ssl/SSLSocketImpl.java b/src/share/classes/sun/security/ssl/SSLSocketImpl.java index db11db8a1..b9f229931 100644 --- a/src/share/classes/sun/security/ssl/SSLSocketImpl.java +++ b/src/share/classes/sun/security/ssl/SSLSocketImpl.java @@ -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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { */ 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 // @@ -653,6 +659,17 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { // READING AND WRITING RECORDS // + /* + * 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. @@ -660,7 +677,7 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { * 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 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { try { if (writeLock.tryLock(getSoLinger(), TimeUnit.SECONDS)) { try { - writeRecordInternal(r); + writeRecordInternal(r, holdRecord); } finally { writeLock.unlock(); } @@ -779,7 +796,7 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { } else { writeLock.lock(); try { - writeRecordInternal(r); + writeRecordInternal(r, holdRecord); } finally { writeLock.unlock(); } @@ -787,11 +804,29 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { } } - 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 -- GitLab