Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear sockets perform worse than TLS socket in some cases #27970

Open
tufosa opened this issue May 30, 2019 · 7 comments
Open

Clear sockets perform worse than TLS socket in some cases #27970

tufosa opened this issue May 30, 2019 · 7 comments
Labels
performance Issues and PRs related to the performance of Node.js.

Comments

@tufosa
Copy link

tufosa commented May 30, 2019

While writing and performing tests for this issue opened by @alexfernandez and fixed by @jmendeth, I noticed that in some cases, clear connections seem to perform worse than TLS connections, specially after the fix that enhances significantly the performance of the TLS connections. I will patch the existing secure-pair benchmark test to expose the issue.

The performance of clear connections seems to be the same across node versions (I've tested it in node 8, 10 and 13-pre). This means that this issue is not a regression, but an enhancement, and the fix needed is probably very similar to the one that solved the secure-pair performance issue.

tufosa pushed a commit to tufosa/node that referenced this issue May 30, 2019
adds clear connections to the secure-pair performance test to prove
thah in some cases (when the sender send the data in small chunks)
clear connections perform worse than TLS connections

Refs: nodejs#27970
@tufosa
Copy link
Author

tufosa commented May 30, 2019

I've submitted a PR that includes clear connections in the secure-pair performance test and exposes this issue. The following table shows the results of the test run on my machine.

packet size MBs transferred(TLS) MBs transferred(clear)
2 99.3 2.9
100 1401.76 552.05
1024 1611.28 3231.61
1024 * 1024 2519.03 7797.26

gireeshpunathil pushed a commit that referenced this issue Dec 20, 2019
adds clear connections to the secure-pair performance test to prove
that in some cases (when the sender send the data in small chunks)
clear connections perform worse than TLS connections

Also add a byte chunk size test to benchmark/net/net-pipe.js

Refs: #27970

PR-URL: #27971
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
BridgeAR pushed a commit that referenced this issue Jan 3, 2020
adds clear connections to the secure-pair performance test to prove
that in some cases (when the sender send the data in small chunks)
clear connections perform worse than TLS connections

Also add a byte chunk size test to benchmark/net/net-pipe.js

Refs: #27970

PR-URL: #27971
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this issue Jan 14, 2020
adds clear connections to the secure-pair performance test to prove
that in some cases (when the sender send the data in small chunks)
clear connections perform worse than TLS connections

Also add a byte chunk size test to benchmark/net/net-pipe.js

Refs: #27970

PR-URL: #27971
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
adds clear connections to the secure-pair performance test to prove
that in some cases (when the sender send the data in small chunks)
clear connections perform worse than TLS connections

Also add a byte chunk size test to benchmark/net/net-pipe.js

Refs: #27970

PR-URL: #27971
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@jasnell jasnell added the performance Issues and PRs related to the performance of Node.js. label Jun 26, 2020
@jasnell
Copy link
Member

jasnell commented Jun 26, 2020

It's not clear if there's anything further actionable here.

@mildsunrise
Copy link
Member

When constantly writing tiny buffers, TLS connections are much faster now because we concatenate them before passing to OpenSSL. For plaintext connections, all those buffers are sent directly to the kernel which seems to incur this overhead.

Maybe concatenating would be possible for plaintext sockets too? @nodejs/net

However I want to point out this seems like an artificial test case to me. When moving lots of data, chunks are unlikely to be tiny. And if they are tiny, you could easily concatenate them yourself in bigger ones.

@alexfernandez
Copy link

This test case tries to reproduce what @tufosa and I found at Devo: we used Node.js to receive syslog streams over a socket, and we usually received lots of small packets. It is not always that you have the luxury of deciding what packet size customers send you 😅

Concatenating would be ideal for this particular application, and it would speed up reception for everyone else, right?

@addaleax
Copy link
Member

We already support writev() for network sockets, so the next thing I could think off would be enabling Nagle’s algorithm where we currently disable it (e.g. HTTP/2), or build our own version of it?

@jasnell
Copy link
Member

jasnell commented Jun 26, 2020

With HTTP/2 we really should not enable Nagle at all, but it can certainly be enabled elsewhere.

@mildsunrise
Copy link
Member

IIRC we use writev and Nagle by default, something else may be causing this overhead. I'm just speculating, but I think passing tens of thousands of tiny buffers around (to libuv and the kernel) may be causing it. If I'm right, maybe not using writev (concatenating instead) would help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Development

No branches or pull requests

5 participants