http: optimize by corking outgoing requests

lib/_http_outgoing.js

@@ -83,6 +83,10 @@ function OutgoingMessage() {
   this._headers = null;
   this._headerNames = {};
 
+  // When this is written to, it will cork for a tick. This represents
+  // whether the socket is corked for that reason or not.
+  this._corkedForWrite = false;
+
   this._onPendingData = null;
 }
 util.inherits(OutgoingMessage, Stream);
@@ -112,33 +116,26 @@ OutgoingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
 // any messages, before ever calling this.  In that case, just skip
 // it, since something else is destroying this connection anyway.
 OutgoingMessage.prototype.destroy = function destroy(error) {
-  if (this.socket)
+  if (this.socket) {
+    // If the socket is corked from a write, uncork it before destroying it.
+    if (this._corkedForWrite)
+      this.socket.uncork();
+
     this.socket.destroy(error);
-  else
+  } else {
     this.once('socket', function(socket) {
       socket.destroy(error);
     });
+  }
 };
 
 
 // This abstract either writing directly to the socket or buffering it.
 OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
-  // This is a shameful hack to get the headers and first body chunk onto
-  // the same packet. Future versions of Node are going to take care of
-  // this at a lower level and in a more general way.
+  // Send the headers before the body. OutgoingMessage#write will cork
+  // the stream before calling #_send, batching them in the same packet.
   if (!this._headerSent) {
-    if (typeof data === 'string' &&
-        encoding !== 'hex' &&
-        encoding !== 'base64') {
-      data = this._header + data;
-    } else {
-      this.output.unshift(this._header);
-      this.outputEncodings.unshift('latin1');
-      this.outputCallbacks.unshift(null);
-      this.outputSize += this._header.length;
-      if (typeof this._onPendingData === 'function')
-        this._onPendingData(this._header.length);
-    }
+    this._writeRaw(this._header, 'latin1', null);
     this._headerSent = true;
   }
   return this._writeRaw(data, encoding, callback);
@@ -158,10 +155,10 @@ function _writeRaw(data, encoding, callback) {
       connection.writable &&
       !connection.destroyed) {
     // There might be pending data in the this.output buffer.
-    var outputLength = this.output.length;
-    if (outputLength > 0) {
-      this._flushOutput(connection);
-    } else if (data.length === 0) {
+    this._flushOutput(connection);
+
+    // Avoid writing empty messages, but trigger the callback.
+    if (data.length === 0) {
       if (typeof callback === 'function')
         process.nextTick(callback);
       return true;
@@ -465,31 +462,26 @@ OutgoingMessage.prototype.write = function write(chunk, encoding, callback) {
   // signal the user to keep writing.
   if (chunk.length === 0) return true;
 
+  // By corking the socket and uncorking after a tick, we manage to
+  // batch writes together, i.e. the header with the body.
+  if (this.connection && !this._corkedForWrite) {
+    this.connection.cork();
+    this._corkedForWrite = true;
+    process.nextTick(connectionCorkNT, this);
+  }
+
   var len, ret;
   if (this.chunkedEncoding) {
-    if (typeof chunk === 'string' &&
-        encoding !== 'hex' &&
-        encoding !== 'base64' &&
-        encoding !== 'latin1') {
+    if (typeof chunk === 'string') {
       len = Buffer.byteLength(chunk, encoding);
-      chunk = len.toString(16) + CRLF + chunk + CRLF;
-      ret = this._send(chunk, encoding, callback);
     } else {
-      // buffer, or a non-toString-friendly encoding
-      if (typeof chunk === 'string')
-        len = Buffer.byteLength(chunk, encoding);
-      else
-        len = chunk.length;
-
-      if (this.connection && !this.connection.corked) {
-        this.connection.cork();
-        process.nextTick(connectionCorkNT, this.connection);
-      }
-      this._send(len.toString(16), 'latin1', null);
-      this._send(crlf_buf, null, null);
-      this._send(chunk, encoding, null);
-      ret = this._send(crlf_buf, null, callback);
+      len = chunk.length;
     }
+
+    this._send(len.toString(16), 'latin1', null);
+    this._send(crlf_buf, null, null);
+    this._send(chunk, encoding, null);
+    ret = this._send(crlf_buf, null, callback);
   } else {
     ret = this._send(chunk, encoding, callback);
   }
@@ -505,8 +497,9 @@ function writeAfterEndNT(self, err, callback) {
 }
 
 
-function connectionCorkNT(conn) {
-  conn.uncork();
+function connectionCorkNT(msg) {
+  msg.connection.uncork();
+  msg._corkedForWrite = false;
 }
 
 

test/parallel/test-http-write-write-destroy.js

@@ -0,0 +1,57 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+// Ensure that, in a corked response, calling .destroy() will flush what was
+// previously written completely rather than partially or none at all. It
+// also makes sure it's written in a single packet.
+
+var hasFlushed = false;
+
+var server = http.createServer(common.mustCall((req, res) => {
+  res.write('first.');
+  res.write('.second', function() {
+    // Set the flag to prove that all data has been written.
+    hasFlushed = true;
+  });
+
+  res.destroy();
+
+  // If the second callback from the .write() calls hasn't executed before
+  // the next tick, then the write has been buffered and was sent
+  // asynchronously. This means it wouldn't have been written regardless of
+  // corking, making the test irrelevant, so skip it.
+  process.nextTick(function() {
+    if (hasFlushed) return;
+
+    common.skip('.write() executed asynchronously');
+    process.exit(0);
+    return;
+  });
+}));
+
+server.listen(0);
+
+server.on('listening', common.mustCall(() => {
+  // Send a request, and assert the response.
+  http.get({
+    port: server.address().port
+  }, (res) => {
+    var data = '';
+
+    // By ensuring that the 'data' event is only emitted once, we ensure that
+    // the socket was correctly corked and the data was batched.
+    res.on('data', common.mustCall(function(chunk) {
+      data += chunk.toString('latin1');
+    }, 2));
+
+    res.on('end', common.mustCall(function() {
+      assert.equal(data, 'first..second');
+
+      res.destroy();
+      server.close();
+    }));
+  });
+}));