X Tutup
The Wayback Machine - https://web.archive.org/web/20201023141359/https://github.com/nodejs/node/pull/35753
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

tls: allow reading data into a static buffer #35753

Open
wants to merge 1 commit into
base: master
from

Conversation

@puzpuzpuz
Copy link
Member

@puzpuzpuz puzpuzpuz commented Oct 22, 2020

Refs: #25436

Adds onread option support into tls.connect(). The option is only used when the socket is not specified. The end behavior is the same as with net.connect()'s onread option. Just like with the net's option, it's a client side feature.

Also, adds new benchmark/tls/throughput-s2c.js benchmark to test the option and renames the existing throughput.js benchmark into throughput-c2s.js to have consistent naming.

Refs: #25436

Benchmark results (here recvbuflen=0 stands for the current behavior of allocating a new buffer for each chunk read):

tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=0 sendchunklen=256 type="buf" dur=5: 3,462.644485289712
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=65536 sendchunklen=256 type="buf" dur=5: 3,908.724691726506
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=1048576 sendchunklen=256 type="buf" dur=5: 3,880.9003883777
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=0 sendchunklen=32768 type="buf" dur=5: 6,609.7547144796
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=65536 sendchunklen=32768 type="buf" dur=5: 7,875.555400944821
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=1048576 sendchunklen=32768 type="buf" dur=5: 7,853.576701856802
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=0 sendchunklen=131072 type="buf" dur=5: 7,125.125787317291
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=65536 sendchunklen=131072 type="buf" dur=5: 8,429.701028374991
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=1048576 sendchunklen=131072 type="buf" dur=5: 8,512.095293725759
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=0 sendchunklen=16777216 type="buf" dur=5: 6,781.770526905906
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=65536 sendchunklen=16777216 type="buf" dur=5: 7,972.510549406974
tls/throughput-s2c.js recvbufgenfn="false" recvbuflen=1048576 sendchunklen=16777216 type="buf" dur=5: 7,958.803175049797
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
@nodejs-github-bot
Copy link

@nodejs-github-bot nodejs-github-bot commented Oct 22, 2020

Review requested:

@puzpuzpuz puzpuzpuz force-pushed the puzpuzpuz:enhancement/reuse-tls-read-buffer branch from 3397b90 to 12c6ad0 Oct 22, 2020
@puzpuzpuz puzpuzpuz force-pushed the puzpuzpuz:enhancement/reuse-tls-read-buffer branch from 12c6ad0 to 15dcd4b Oct 22, 2020
@puzpuzpuz puzpuzpuz marked this pull request as ready for review Oct 22, 2020
Copy link
Member

@mcollina mcollina left a comment

lgtm

@codecov-io

This comment has been hidden.

@puzpuzpuz
Copy link
Member Author

@puzpuzpuz puzpuzpuz commented Oct 22, 2020

It looks like codecov is living its own life. I'm failing to understand how the changes from this PR may decrease code coverage by 8.50%. The impacted file list also looks suspicious. I'll create an issue to gather some insights.

@puzpuzpuz
Copy link
Member Author

@puzpuzpuz puzpuzpuz commented Oct 23, 2020

Marked codecov message as hidden. See #35759 (comment) for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.
X Tutup