Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[tls] write data by small (1500 bytes) chunks, see http://www.belshe.com/ #961

Closed
wants to merge 2 commits into from

6 participants

@indutny
Owner

This pull request was previously posted here, but this time it's low-level:

P.S.
See http://www.belshe.com/2010/12/17/performance-and-the-tls-record-size/

@ry
ry commented

benchmark?

@indutny
Owner

Sorry, no benchmarks. That is optimization for server client's - read article carefully! (Last time I'd posted that pull request - I missed that fact)

@indutny
Owner

Basically, if you're sending large chunks - browser can display parts of page before it stop receiving, b/c part of chunk can't be encrypted.

@indutny
Owner

BTW, try this in Chrome:
https://donnerjack13589.dyndns.tv:8081/

It's nodejs SPDY server :) And it's sending data in 1500 bytes chunks

@pquerna
Owner

I'm concerned about the effect this might have on throughput, and thats what it would be nice to have a benchmark of -- showing the effect is negligible.

1500 byte writes to SSL_write also would cause a bad behavior for many connections, because that would become 1500+ bytes with the TLS headers/protocol, and for most MTUs would end up being 2 packets instead of 1. (good thing to look at in wireshark over non-localhost)

If the benchmark shows this isn't a huge drop in throughput, I'd be interested in seeing what happens with using ~1300-1400 byte writes, which should drop you bellow the common MTUs.

@indutny
Owner

Hm... Nice point, @pquerna.

Mike has told me to use 1400 bytes, but I'd seen 1500 in chromium's flip_server - so I decided to use last one...
Probably that was wrong step.

Can you please help and perform some tests?

@indutny
Owner

I think we should come back to this issue, splitting can't be done in userland

@indutny
Owner

Reconsider?

@Nodejs-Jenkins

Can one of the admins verify this patch?

@indutny
Owner

fuck it.

@indutny indutny closed this
@isaacs
Owner

fuck it.

@indutny you should be more friendly to pull request submitters ;P

@AlexeyKupershtokh

Is it a special command for jenkins?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 36 additions and 7 deletions.
  1. +2 −4 lib/tls.js
  2. +34 −3 src/node_crypto.cc
View
6 lib/tls.js
@@ -382,8 +382,8 @@ CryptoStream.prototype._pull = function() {
this.pair._maybeInitFinished();
- if (rv === 0 || rv < 0) {
- this._pending.unshift(tmp);
+ if (rv < tmp.length) {
+ this._pending.unshift(tmp.slice(Math.max(rv, 0)));
this._pendingCallbacks.unshift(cb);
break;
}
@@ -392,8 +392,6 @@ CryptoStream.prototype._pull = function() {
assert(this._pendingBytes >= 0);
if (cb) cb();
-
- assert(rv === tmp.length);
}
// If we've cleared all of incoming encrypted data, emit drain.
View
37 src/node_crypto.cc
@@ -959,12 +959,43 @@ Handle<Value> Connection::ClearIn(const Arguments& args) {
if (rv < 0) return scope.Close(Integer::New(rv));
}
- int bytes_written = SSL_write(ss->ssl_, buffer_data + off, len);
+ const int chunkSize = 1500;
+ int rv = 0;
+ int offset = 0;
+ int total_bytes_written = 0;
+
+ while (len > 0) {
+ int chunkSize_ = chunkSize > len ? len : chunkSize;
+ int bytes_written = SSL_write(ss->ssl_,
+ buffer_data + off + offset,
+ chunkSize_);
+ bytes_written = ss->HandleSSLError("SSL_write:ClearIn",
+ bytes_written);
+
+ // Written nothing at all
+ if (bytes_written <= 0) {
+ // If we has failed to write first chunk
+ // total_bytes_written can have negative value
+ if (total_bytes_written == 0) {
+ total_bytes_written += bytes_written;
+ }
+ break;
+ }
+
+ total_bytes_written += bytes_written;
+
+ // Written less than planned
+ if (bytes_written < chunkSize_) {
+ break;
+ }
+
+ offset += chunkSize_;
+ len -= chunkSize_;
+ }
- ss->HandleSSLError("SSL_write:ClearIn", bytes_written);
ss->SetShutdownFlags();
- return scope.Close(Integer::New(bytes_written));
+ return scope.Close(Integer::New(total_bytes_written));
}
Something went wrong with that request. Please try again.