From 42a94e810476aa73d71bfaae9d41610b5d72ef21 Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Tue, 15 Sep 2015 17:37:35 -0700 Subject: [PATCH 1/2] net: cork socket while connecting Instead of allowing the socket to lazily buffer up a write while connecting, proactively .cork() the stream and .uncork() when we have connected. This allows the stream to buffer together all writes while connecting into a single writev when connected, rather than an initial write and a follow-up writev. This only leads us to a small caveat: if the stream is uncorked while connecting, writes will begin to be sent to the socket unintentionally. Work around this by preserving a smaller subset of the _pendingData and _pendingEncoding stuff, which can be used in this case. (This also happens when .end() is called on a socket which is still connecting.) This means this will be turned into a single write call: var socket = net.connect(...); socket.write('hello, wor'); socket.write('ld!'); Improving performance when this is the case. --- lib/net.js | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/net.js b/lib/net.js index c5a5b357acd7b8..5e4f5634d262e2 100644 --- a/lib/net.js +++ b/lib/net.js @@ -154,8 +154,9 @@ function Socket(options) { initSocketHandle(this); - this._pendingData = null; - this._pendingEncoding = ''; + // Buffer for the single possible write while connecting: when uncorked + // or ended, then written to. Writable will buffer any other writes. + this._pendingWrite = null; // handle strings directly this._writableState.decodeStrings = false; @@ -628,19 +629,18 @@ Socket.prototype.write = function(chunk, encoding, cb) { Socket.prototype._writeGeneric = function(writev, data, encoding, cb) { - // If we are still connecting, then buffer this for later. - // The Writable logic will buffer up any more writes while - // waiting for this one to be done. + // If the socket is ended / uncorked while connecting, a single write can + // fire through the cork. Buffer it and send it once connected. Writable + // will buffer any sequential writes if uncorked. if (this._connecting) { - this._pendingData = data; - this._pendingEncoding = encoding; + this._pendingWrite = { data: data, encoding: encoding }; + this.once('connect', function() { + this._pendingWrite = null; this._writeGeneric(writev, data, encoding, cb); }); return; } - this._pendingData = null; - this._pendingEncoding = ''; this._unrefTimer(); @@ -730,8 +730,6 @@ function createWriteReq(req, handle, data, encoding) { Socket.prototype.__defineGetter__('bytesWritten', function() { var bytes = this._bytesDispatched; const state = this._writableState; - const data = this._pendingData; - const encoding = this._pendingEncoding; if (!state) return undefined; @@ -743,11 +741,13 @@ Socket.prototype.__defineGetter__('bytesWritten', function() { bytes += Buffer.byteLength(el.chunk, el.encoding); }); - if (data) { - if (data instanceof Buffer) - bytes += data.length; - else - bytes += Buffer.byteLength(data, encoding); + var buffer = this._pendingWrite; + if (buffer) { + if (buffer.data instanceof Buffer) { + bytes += buffer.data.length; + } else { + bytes += Buffer.byteLength(buffer.data, buffer.encoding); + } } return bytes; @@ -869,6 +869,7 @@ Socket.prototype.connect = function(options, cb) { } if (this.destroyed) { + // Recycle the socket. this._readableState.reading = false; this._readableState.ended = false; this._readableState.endEmitted = false; @@ -899,6 +900,12 @@ Socket.prototype.connect = function(options, cb) { this._connecting = true; this.writable = true; + // Cork then uncork upon connecting. + this.cork(); + this.once('connect', function() { + this.uncork(); + }); + if (pipe) { connect(this, options.path); } else { From 63e3030df9d22e25c6b492dd5dcfe9bb2859ba18 Mon Sep 17 00:00:00 2001 From: Brendan Ashworth Date: Fri, 18 Sep 2015 16:38:24 -0700 Subject: [PATCH 2/2] net: fix bytesWritten during writev When a writev is caused on a socket (sometimes through corking and uncorking), previously net would call Buffer.byteLength on the array of buffers and chunks, giving a completely erroneous byte length for the bulk of them (this is because byteLength is weird and coerces to strings). This commit fixes this bug by iterating over each chunk in the pending stack and calculating the length individually. Also adds a regression test. --- lib/net.js | 13 ++++++- test/parallel/test-net-socket-byteswritten.js | 36 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-net-socket-byteswritten.js diff --git a/lib/net.js b/lib/net.js index 5e4f5634d262e2..bb0072fa736cf7 100644 --- a/lib/net.js +++ b/lib/net.js @@ -742,7 +742,18 @@ Socket.prototype.__defineGetter__('bytesWritten', function() { }); var buffer = this._pendingWrite; - if (buffer) { + if (Array.isArray(buffer)) { + // was a writev, iterate over chunks to get total length + for (let i = 0; i < buffer.length; i++) { + let chunk = buffer[i]; + + if (chunk instanceof Buffer) { + bytes += chunk.length; + } else { + bytes += Buffer.byteLength(chunk.data, chunk.encoding); + } + } + } else if (buffer) { if (buffer.data instanceof Buffer) { bytes += buffer.data.length; } else { diff --git a/test/parallel/test-net-socket-byteswritten.js b/test/parallel/test-net-socket-byteswritten.js new file mode 100644 index 00000000000000..3a8b40f6f605bd --- /dev/null +++ b/test/parallel/test-net-socket-byteswritten.js @@ -0,0 +1,36 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); +const Buffer = require('buffer').Buffer; + +var server = net.createServer(function(socket) { + socket.end(); +}); + +server.listen(common.PORT, common.mustCall(function() { + var socket = net.connect(common.PORT); + + // Cork the socket, then write twice; this should cause a writev, which + // previously caused an err in the bytesWritten count. + socket.cork(); + + socket.write('one'); + socket.write(new Buffer('twø', 'utf8')); + + socket.uncork(); + + // one = 3 bytes, twø = 4 bytes + assert.equal(socket.bytesWritten, 3 + 4); + + socket.on('connect', common.mustCall(function() { + assert.equal(socket.bytesWritten, 3 + 4); + })); + + socket.on('end', common.mustCall(function() { + assert.equal(socket.bytesWritten, 3 + 4); + + server.close(); + })); +}));