net: enable TCP_NODELAY by default on all platforms #906

I may have to dig deeper here. Windows for example doesn't default to TCP_NODELAY either, so it seems questionable if we would rely in platform defaults like the issue implies.

+1 seems like a bug, my initial thought was the docs should be changed but it seems like nodelay is highly recommended as default for today's networking. I would hazard a guess that os platforms not already doing this are playing it safe where as iojs can be more reactive to current practice.

Yes, Nagle's algorithm isn't really beneficial in today's latency-limited networks. I wonder if it's possible to test from JS whether it is enabled on a target host.

+1

+1 from me too, I'd love to see this.

The fact that the documentation says that TCP_NODELAY is enabled by default seems like a documentation bug to me. As far as I know, that has never been true.

I'm on the fence as to whether it's a good idea to enable it by default. I'd be more comfortable +1'ing that if I had more faith in the net module's capabilities of batching writes. That's not to say I object, just that I'm not sure if defaulting to TCP_NODELAY is an unequivocally good thing.

If we go through with it enabling it everywhere, we need an mechanism for the users to disable it globally too. I think ideally on a per-process basis, maybe something like process.setNoDelay().

The reason is simple: With net only exposing this option on single sockets, there no way every module is going to expose the option to its parent.

I think regardless if we enable it by default or not, a process.setNoDelay() could generally prove useful.

While it can be set at kernel-level, I think controlling it on a per-program basis is preferred. For example, nginx has the option to set it per-server.

@bnoordhuis I wonder if adding a net.setNoDelay() to allow the default to be changed would be useful. Users in the field who are trying to squeeze more performance out could report back if any of them actually find the feature useful. If noone finds it useful to change no delay globally, there's no reason to change the default.

A net.setNoDelay() was my first idea too, but if I understand this correctly, that would require the user to require('net') before anything else to work on all connections following the call. We probably don't want the require order to be significant.

Or would it be possible to apply TCP_NODELAY retroactively to all sockets of a program?

I generally dislike any type of "global" setting. You wanted to speed up your http server but without realizing the database driver just got slower.

Node should pick reasonable defaults (so maybe switch it on by default), behave consistently on all platforms (fix windows and/or aix), and be easy to configure (maybe setNoDelay is too difficult for http servers). Let's keep bikeshedding withing this parameter space.

As a first step, I'm trying to verify that setNoDelay is working when set on both client and server:

"use strict";
const net = require("net");
const port = 4000;
let timeClient, timeServer;

function time() {
    let hrtime = process.hrtime();
    return hrtime[0] * 1e9 + hrtime[1];
}

net.createServer(function(socket) {
    socket.setNoDelay(true); // correct?
    timeServer = time();
    socket.write("\0");
    socket.on("data", function () {
        console.log("Client -> Server " + ((time() - timeClient) / 1000).toFixed(0) + "µs");
    });
}).listen(port);

setInterval(function() {
    let socket = new net.Socket();
    socket.setNoDelay(true); // correct?
    socket.on("data", function () {
        console.log("Server -> Client " + ((time() - timeServer) / 1000).toFixed(0) + "µs");
        timeClient = time();
        socket.write("\0");
        socket.end();
    }).connect(port);
}, 200);

Is this the correct usage? @evanlucas maybe you can have a look, as you've dealt with these socket options recently.

edit: updated to measure both delays. also: i fail at math, it's µs :)
edit2: I probably need to send out writes way faster to notice the nagling.

@silverwind net.Socket#setNoDelay() is a no-op when the socket isn't connected yet. Changing the call to socket.once('connect', socket.setNoDelay) should work around that.

The current benchmarking setup only supports Linux x86. Does anybody on this discussion know if TCP_NODELAY is enabled/disabled by default on this platoform. I can search for the answer, but if anybody already know based on the past discussion it would same me some time.

@mhdawson TCP_NODELAY is disabled by default on Linux and there is no sysctl to override that, as far as I'm aware. A quick check of net/ipv4/tcp.c seems to confirm that.

ping @mhdawson ^

Applied this patch: https://github.com/mhdawson/io.js/commit/eb83dc31db2f68a67473dc6fdbcd521994d2b446.patch

Initial results

BEFORE CHANGE: https://ci.nodejs.org/view/All/job/benchmark-footprint-experimental-TCP_NODELAY/1/

+ cat acmerun
+ grep metric throughput 
+ awk {print $3}
+ ACME_THROUGHPUT=2330.13
+ cat acmerun
+ grep metric latency
+ awk {print $3}
+ ACME_LATENCY=10.2826
+ cat acmerun
+ grep metric pre footprint
+ awk {print $4}
+ ACME_PREFOOTPRINT=103296
+ cat acmerun
+ grep metric post footprint
+ awk {print $4}
+ ACME_POSTFOOTPRINT=98368

AFTER CHANGE: https://ci.nodejs.org/view/All/job/benchmark-footprint-experimental-TCP_NODELAY/5/consoleFull

+ cat acmerun
+ grep metric throughput 
+ awk {print $3}
+ ACME_THROUGHPUT=2331.48
+ cat acmerun
+ grep metric latency
+ awk {print $3}
+ ACME_LATENCY=10.2697
+ cat acmerun
+ grep metric pre footprint
+ awk {print $4}
+ ACME_PREFOOTPRINT=105388
+ cat acmerun
+ + grepawk {print $4}
 metric post footprint
+ ACME_POSTFOOTPRINT=100800

assuming that it was off by default and that my patch properly turns it on where it matters, it does not seem to have affected latency or throughput to a noticeable degree. More runs would like be needed to confirm.

@mhdawson what packet size are you using in these tests? Can you try to vary them?

Do you have the command to vary the packet sizes from the command line ? If so I can launch some runs with the values you'd like.

On Linux, it should be ip link set dev eth0 mtu <bytes> where bytes is 0-1500.

Should this remain open?

I'd say so.

I just stumbled upon this and I know that using the NAGL algorithm has a huge impact in case you send very tiny buffer chunks and wait for the result as done in the node_redis benchmarks.

Results

// No NAGL
   SET 4B buf,         1/1 avg/max:   0.05/  3.07 2501ms total,   18224 ops/sec
// NAGL
   SET 4B buf,         1/1 avg/max:  44.10/ 47.97 2514ms total,      23 ops/sec

So I definitely think this is a good idea. I do not know of any negative side effects but I only checked this for node_redis.

@BridgeAR do you think you could come up with a test that verifies that it's enabled? This was the part that I was struggling with last time. Other than that, the change itself to enable it should be just setting the socket option.

ping @BridgeAR

Over a year with no update, so I'm going to close this.
Feel free to ping me if this should be reopened.