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

diff --git a/folly/io/async/AsyncSSLSocket.cpp b/folly/io/async/AsyncSSLSocket.cpp
index da5ca85ecc1edf8bdc2559b243f537a8a3b409fa..a6e3307cb832ddea24297f0292e09ba3540f381a 100644 (file)
--- 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 c3d52150a3b21aebcbf4327092f964c3fc2d27fe..ea7cbbdd5998b84143596702c5ee547a473aedd3 100644 (file)
--- 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 bc899791818c5236e56523534fcb5b41561aa93a..5f1ca7304083b800fa45cc1ca06844b41b41e556 100644 (file)
--- 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>();
+  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());
+}
 }
 
 ///////////////////////////////////////////////////////////////////////////