lib: introduce .setMaxSendFragment(size)

[indutny]

fix #6889

[igrigorik]

It would be great to use a lower max size. Ideally, it would be adjusted over the lifetime of the connection, but at a minimum, we want to eliminate the CWND overlfow case on new connections + slow-start after idle cases... Setting fragment size to ~8k would do the trick (assuming we're on IW10 server).

[indutny]

It affects throughput significantly, I did benchmarks of bud with and without this option enabled and the difference was dramatical. Also, default value is set by OpenSSL, not us.

[igrigorik]

Out of curiosity, do you have your bud benchmark numbers published anywhere?

[indutny]

Yep, these are with max send fragment set to nearly MTU value (~1300 or a bit less): indutny/bud#20 (comment)

And here are results without max send fragment:

indutny/bud#20 (comment)

[indutny]

There're no bandwidth metric, so you could only believe me and observe lower RPS :)

[igrigorik]

I don't think that we are going to implement dynamic strategy in core.

Do you have any particular reason in mind? Config flag is definitely a good start (and kudos for super-quick turnaround!), but I'd love to see a smarter strategy in node.

[tjfontaine]

Sometimes it's not always in our best interest to implement (and then maintain) optimizations around specific implementations that could be subject to bikeshedding. Ideally we're working to provide APIs in core that allow external consumers to handle these features in the module space.

By simply making this configurable, it allows someone to publish the auto-window management without Node being opinionated about what that means.

[igrigorik]

@tjfontaine I guess from a framework perspective that approach makes sense. That said, my reservation with this line of thinking is the suboptimal "out-of-the-box" experience: most developers won't know to enable an extra module or add that extra config flag, hence the majority of deployed servers will continue to use the suboptimal settings and users will incur higher TTFB than necessary.

The advantage of the dynamic strategy is that you can meet both goals: optimize TTFB for interactive traffic, and provide good through for bulk transfers. No configuration needed, unless you want to tweak the boost threshold.

[tjfontaine]

@igrigorik I understand your sentiment, but more often than not deployments are using servers like restify, hapi, or express -- each with their own priorities regarding what they're designed to provide. They are in a much better place to define the policy and mechanism they want to adopt. They could even use the module system to share implementations among each other. This is the ethos of core and of the ecosystem in general.

As far as TTFB considerations for people using require('https').createServer(), that should be noted in our documentation of the method setMaxSendFragment such that users can be aware and be pointed to more in depth information that can be utilized.

[igrigorik]

@indutny @tjfontaine sg.

What do you guys think of the following for the documentation:

Set maximum TLS fragment size (default value is: 16384). Smaller fragment size decreases buffering latency on the client: large fragments are buffered by the TLS layer until the entire fragment is received and its integrity is verified; large records can span multiple roundtrips, and their processing can be delayed due to packet loss or reordering. However, smaller record size adds extra TLS framing bytes and CPU overhead, which may decrease overall server throughput.

Not sure what the policy is about linking to external sites in the doc, but if it makes sense:

• http://chimera.labs.oreilly.com/books/1230000000545/ch04.html#TLS_RECORD_SIZE
• http://www.igvita.com/2013/10/24/optimizing-tls-record-size-and-buffering-latency/

[indutny]

That is very meaningful documentation, thank you! Though, I'll insert it without links.

[igrigorik]

@indutny sg, thanks!

[indutny]

Pushed update.

[indutny]

@igrigorik fixed.

[indutny]

@tjfontaine please review

[tjfontaine]

will do

[tjfontaine]

Can we add test coverage on this part? Is there an upper bound?

[indutny]

Yep 16384, will add test tomorrow.

[indutny]

Added test coverage.

[tjfontaine]

lgtm

[indutny]

Thank you, landed in 7f9b015