From 12f0c056b4f3ecdfaa4cb3f10ed0d5f133e91f75 Mon Sep 17 00:00:00 2001 From: Jun Li Date: Wed, 22 Apr 2015 14:09:07 -0700 Subject: [PATCH] Make MIN_WRITE_SIZE configurable for AsyncSSLSocket. Summary: MIN_WRITE_SIZE is used to avoid small write calls to SSL_write. If there are consecutive small buffers to write, then will be combined together(by being copied to a local buffer) until total size exceeds MIN_WRITE_SIZE. This reduces number of calls SSL_write, improving performance, and avoiding overhead in OpenSSL. Currently, MIN_WRITE_SIZE is hard coded to be 1500 bytes. Wormhole could benefit from this, as our average message size is several hundreds of bytes. We could get even better throughput with larger MIN_WRITE_SIZE. As discussed with Adam and Alan, there is a good reason to make it configurable, though default value is still 1500. Test Plan: unit tests Reviewed By: simpkins@fb.com Subscribers: net-systems@, ssl-diffs@, folly-diffs@, yfeldblum, chalfant, thomasf FB internal diff: D1996570 Tasks: 6784543 Signature: t1:1996570:1429667035:a661ef30a715dafec3e134a7f6af6f56ada2e8e0 --- folly/io/async/AsyncSSLSocket.cpp | 37 ++++++++++++++++++---- folly/io/async/AsyncSSLSocket.h | 12 +++++++ folly/io/async/test/AsyncSSLSocketTest.cpp | 18 +++++++++++ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/folly/io/async/AsyncSSLSocket.cpp b/folly/io/async/AsyncSSLSocket.cpp index da5ca85e..a6e3307c 100644 --- a/folly/io/async/AsyncSSLSocket.cpp +++ b/folly/io/async/AsyncSSLSocket.cpp @@ -56,9 +56,6 @@ using folly::AsyncSocketException; using folly::AsyncSSLSocket; using folly::Optional; -/** Try to avoid calling SSL_write() for buffers smaller than this: */ -size_t MIN_WRITE_SIZE = 1500; - // We have one single dummy SSL context so that we can implement attach // and detach methods in a thread safe fashion without modifying opnessl. static SSLContext *dummyCtx = nullptr; @@ -68,6 +65,10 @@ static SpinLock dummyCtxLock; const uint8_t TASYNCSSLSOCKET_F_PERFORM_READ = 90; const uint8_t TASYNCSSLSOCKET_F_PERFORM_WRITE = 91; +// If given min write size is less than this, buffer will be allocated on +// stack, otherwise it is allocated on heap +const size_t MAX_STACK_BUF_SIZE = 2048; + // This converts "illegal" shutdowns into ZERO_RETURN inline bool zero_return(int error, int rc) { return (error == SSL_ERROR_ZERO_RETURN || (rc == 0 && errno == 0)); @@ -1174,6 +1175,17 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec, bool cork = isSet(flags, WriteFlags::CORK); CorkGuard guard(fd_, count > 1, cork, &corked_); + // Declare a buffer used to hold small write requests. It could point to a + // memory block either on stack or on heap. If it is on heap, we release it + // manually when scope exits + char* combinedBuf{nullptr}; + SCOPE_EXIT { + // Note, always keep this check consistent with what we do below + if (combinedBuf != nullptr && minWriteSize_ > MAX_STACK_BUF_SIZE) { + delete[] combinedBuf; + } + }; + *countWritten = 0; *partialWritten = 0; ssize_t totalWritten = 0; @@ -1193,7 +1205,7 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec, ssize_t bytes; errno = 0; uint32_t buffersStolen = 0; - if ((len < MIN_WRITE_SIZE) && ((i + 1) < count)) { + if ((len < minWriteSize_) && ((i + 1) < count)) { // Combine this buffer with part or all of the next buffers in // order to avoid really small-grained calls to SSL_write(). // Each call to SSL_write() produces a separate record in @@ -1202,13 +1214,24 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec, // header and the first part of the response body in two // separate SSL records (even if those two records are in // the same TCP packet). - char combinedBuf[MIN_WRITE_SIZE]; + + if (combinedBuf == nullptr) { + if (minWriteSize_ > MAX_STACK_BUF_SIZE) { + // Allocate the buffer on heap + combinedBuf = new char[minWriteSize_]; + } else { + // Allocate the buffer on stack + combinedBuf = (char*)alloca(minWriteSize_); + } + } + assert(combinedBuf != nullptr); + memcpy(combinedBuf, buf, len); do { // INVARIANT: i + buffersStolen == complete chunks serialized uint32_t nextIndex = i + buffersStolen + 1; bytesStolenFromNextBuffer = std::min(vec[nextIndex].iov_len, - MIN_WRITE_SIZE - len); + minWriteSize_ - len); memcpy(combinedBuf + len, vec[nextIndex].iov_base, bytesStolenFromNextBuffer); len += bytesStolenFromNextBuffer; @@ -1219,7 +1242,7 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec, bytesStolenFromNextBuffer = 0; buffersStolen++; } - } while ((i + buffersStolen + 1) < count && (len < MIN_WRITE_SIZE)); + } while ((i + buffersStolen + 1) < count && (len < minWriteSize_)); bytes = eorAwareSSLWrite( ssl_, combinedBuf, len, (isSet(flags, WriteFlags::EOR) && i + buffersStolen + 1 == count)); diff --git a/folly/io/async/AsyncSSLSocket.h b/folly/io/async/AsyncSSLSocket.h index c3d52150..ea7cbbdd 100644 --- a/folly/io/async/AsyncSSLSocket.h +++ b/folly/io/async/AsyncSSLSocket.h @@ -640,6 +640,14 @@ class AsyncSSLSocket : public virtual AsyncSocket { return clientHelloInfo_.get(); } + void setMinWriteSize(size_t minWriteSize) { + minWriteSize_ = minWriteSize; + } + + size_t getMinWriteSize() { + return minWriteSize_; + } + protected: /** @@ -730,6 +738,10 @@ class AsyncSSLSocket : public virtual AsyncSocket { // Only one app EOR byte can be tracked. size_t appEorByteNo_{0}; + // Try to avoid calling SSL_write() for buffers smaller than this. + // It doesn't take effect when it is 0. + size_t minWriteSize_{1500}; + // When openssl is about to sendmsg() across the minEorRawBytesNo_, // it will pass MSG_EOR to sendmsg(). size_t minEorRawByteNo_{0}; diff --git a/folly/io/async/test/AsyncSSLSocketTest.cpp b/folly/io/async/test/AsyncSSLSocketTest.cpp index bc899791..5f1ca730 100644 --- a/folly/io/async/test/AsyncSSLSocketTest.cpp +++ b/folly/io/async/test/AsyncSSLSocketTest.cpp @@ -1206,6 +1206,24 @@ TEST(AsyncSSLSocketTest, NoClientCertHandshakeError) { EXPECT_FALSE(server.handshakeSuccess_); EXPECT_TRUE(server.handshakeError_); } + +TEST(AsyncSSLSocketTest, MinWriteSizeTest) { + EventBase eb; + + // Set up SSL context. + auto sslContext = std::make_shared(); + sslContext->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH"); + + // create SSL socket + AsyncSSLSocket::UniquePtr socket(new AsyncSSLSocket(sslContext, &eb)); + + EXPECT_EQ(1500, socket->getMinWriteSize()); + + socket->setMinWriteSize(0); + EXPECT_EQ(0, socket->getMinWriteSize()); + socket->setMinWriteSize(50000); + EXPECT_EQ(50000, socket->getMinWriteSize()); +} } /////////////////////////////////////////////////////////////////////////// -- 2.34.1