tls: reduce memory copying and number of BIO buffer allocations

Avoid copying buffers before passing to SSL_write if there
are zero length buffers involved.  Only copy the data when
the buffer has a non zero length.

Send a memory allocation hint to the crypto BIO about how much
memory will likely be needed to be allocated by the next call
to SSL_write.  This makes a single allocation rather than the BIO
allocating a buffer for each 16k TLS segment written.  This
solves a problem with large buffers written over TLS triggering
V8's GC.

PR-URL: #31499
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: rustyconover

benchmark/tls/throughput.js

@@ -3,7 +3,7 @@ const common = require('../common.js');
 const bench = common.createBenchmark(main, {
   dur: [5],
   type: ['buf', 'asc', 'utf'],
-  size: [2, 1024, 1024 * 1024]
+  size: [2, 1024, 1024 * 1024, 4 * 1024 * 1024, 16 * 1024 * 1024]
 });
 
 const fixtures = require('../../test/common/fixtures');

src/node_crypto_bio.cc

@@ -438,6 +438,13 @@ void NodeBIO::TryAllocateForWrite(size_t hint) {
                              kThroughputBufferLength;
     if (len < hint)
       len = hint;
+
+    // If there is a one time allocation size hint, use it.
+    if (allocate_hint_ > len) {
+      len = allocate_hint_;
+      allocate_hint_ = 0;
+    }
+
     Buffer* next = new Buffer(env_, len);
 
     if (w == nullptr) {

src/node_crypto_bio.h

@@ -96,6 +96,21 @@ class NodeBIO : public MemoryRetainer {
     return length_;
   }
 
+  // Provide a hint about the size of the next pending set of writes. TLS
+  // writes records of a maximum length of 16k of data plus a 5-byte header,
+  // a MAC (up to 20 bytes for SSLv3, TLS 1.0, TLS 1.1, and up to 32 bytes
+  // for TLS 1.2), and padding if a block cipher is used.  If there is a
+  // large write this will result in potentially many buffers being
+  // allocated and gc'ed which can cause long pauses. By providing a
+  // guess about the amount of buffer space that will be needed in the
+  // next allocation this overhead is removed.
+  inline void set_allocate_tls_hint(size_t size) {
+    constexpr size_t kThreshold = 16 * 1024;
+    if (size >= kThreshold) {
+      allocate_hint_ = (size / kThreshold + 1) * (kThreshold + 5 + 32);
+    }
+  }
+
   inline void set_eof_return(int num) {
     eof_return_ = num;
   }
@@ -164,6 +179,7 @@ class NodeBIO : public MemoryRetainer {
   Environment* env_ = nullptr;
   size_t initial_ = kInitialBufferLength;
   size_t length_ = 0;
+  size_t allocate_hint_ = 0;
   int eof_return_ = -1;
   Buffer* read_head_ = nullptr;
   Buffer* write_head_ = nullptr;

src/tls_wrap.cc

@@ -585,6 +585,7 @@ void TLSWrap::ClearIn() {
   AllocatedBuffer data = std::move(pending_cleartext_input_);
   crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
 
+  crypto::NodeBIO::FromBIO(enc_out_)->set_allocate_tls_hint(data.size());
   int written = SSL_write(ssl_.get(), data.data(), data.size());
   Debug(this, "Writing %zu bytes, written = %d", data.size(), written);
   CHECK(written == -1 || written == static_cast<int>(data.size()));
@@ -699,8 +700,15 @@ int TLSWrap::DoWrite(WriteWrap* w,
 
   size_t length = 0;
   size_t i;
-  for (i = 0; i < count; i++)
+  size_t nonempty_i = 0;
+  size_t nonempty_count = 0;
+  for (i = 0; i < count; i++) {
     length += bufs[i].len;
+    if (bufs[i].len > 0) {
+      nonempty_i = i;
+      nonempty_count += 1;
+    }
+  }
 
   // We want to trigger a Write() on the underlying stream to drive the stream
   // system, but don't want to encrypt empty buffers into a TLS frame, so see
@@ -744,20 +752,34 @@ int TLSWrap::DoWrite(WriteWrap* w,
   crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
 
   int written = 0;
-  if (count != 1) {
+
+  // It is common for zero length buffers to be written,
+  // don't copy data if there there is one buffer with data
+  // and one or more zero length buffers.
+  // _http_outgoing.js writes a zero length buffer in
+  // in OutgoingMessage.prototype.end.  If there was a large amount
+  // of data supplied to end() there is no sense allocating
+  // and copying it when it could just be used.
+
+  if (nonempty_count != 1) {
     data = env()->AllocateManaged(length);
     size_t offset = 0;
     for (i = 0; i < count; i++) {
       memcpy(data.data() + offset, bufs[i].base, bufs[i].len);
       offset += bufs[i].len;
     }
+
+    crypto::NodeBIO::FromBIO(enc_out_)->set_allocate_tls_hint(length);
     written = SSL_write(ssl_.get(), data.data(), length);
   } else {
     // Only one buffer: try to write directly, only store if it fails
-    written = SSL_write(ssl_.get(), bufs[0].base, bufs[0].len);
+    uv_buf_t* buf = &bufs[nonempty_i];
+    crypto::NodeBIO::FromBIO(enc_out_)->set_allocate_tls_hint(buf->len);
+    written = SSL_write(ssl_.get(), buf->base, buf->len);
+
     if (written == -1) {
       data = env()->AllocateManaged(length);
-      memcpy(data.data(), bufs[0].base, bufs[0].len);
+      memcpy(data.data(), buf->base, buf->len);
     }
   }