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

http: use for...of #30958

Closed
wants to merge 10 commits into from
Closed

http: use for...of #30958

wants to merge 10 commits into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Dec 14, 2019

Searched for regular expression for \([^\.]*.length in lib and made changes in _http_* files

Refs: #30910 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Dec 14, 2019
@nodejs-github-bot
Copy link
Collaborator

lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_server.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm -1 changing how we handle headers. We should stick a for(;;).

lib/_http_client.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Outdated Show resolved Hide resolved
lib/_http_outgoing.js Show resolved Hide resolved
lib/_http_server.js Outdated Show resolved Hide resolved
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 17, 2019
@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Dec 17, 2019

Landed in ddedf8e

@Trott Trott closed this Dec 17, 2019
Trott pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #30958
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #30958
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 17, 2019
@trivikr trivikr deleted the use-for-of-http branch December 18, 2019 16:15
@trivikr
Copy link
Member Author

trivikr commented Dec 19, 2019

Comparison benchmarks of updates which weren't pushed in this PR - in files _http_client.js, _http_outgoing.js and _http_server.js

                                                                                               confidence improvement accuracy (*)    (**)   (***)
 http/check_invalid_header_char.js n=1000000 input=''                                                          0.29 %       ±0.94%  ±1.30%  ±1.79%
 http/check_invalid_header_char.js n=1000000 input='\177'                                                      0.54 %       ±1.13%  ±1.55%  ±2.14%
 http/check_invalid_header_char.js n=1000000 input='\\t\\t\\t\\t\\t\\t\\t\\t\\t\\tFoo bar baz'                 0.18 %       ±0.34%  ±0.47%  ±0.64%
 http/check_invalid_header_char.js n=1000000 input='1'                                                        -0.48 %       ±0.56%  ±0.77%  ±1.05%
 http/check_invalid_header_char.js n=1000000 input='20091'                                                    -0.05 %       ±0.52%  ±0.72%  ±0.98%
 http/check_invalid_header_char.js n=1000000 input='close'                                                    -0.06 %       ±0.57%  ±0.77%  ±1.06%
 http/check_invalid_header_char.js n=1000000 input='en-US'                                                    -0.09 %       ±0.48%  ±0.65%  ±0.89%
 http/check_invalid_header_char.js n=1000000 input='foo\\nbar'                                                -0.54 %       ±1.05%  ±1.46%  ±2.04%
 http/check_invalid_header_char.js n=1000000 input='group_acmeair'                                            -0.21 %       ±0.73%  ±1.02%  ±1.41%
 http/check_invalid_header_char.js n=1000000 input='gzip'                                                     -0.04 %       ±0.63%  ±0.86%  ±1.19%
 http/check_invalid_header_char.js n=1000000 input='keep-alive'                                               -0.04 %       ±0.64%  ±0.89%  ±1.24%
 http/check_invalid_header_char.js n=1000000 input='LONG_AND_INVALID'                                          3.02 %       ±7.04% ±10.11% ±14.87%
 http/check_invalid_header_char.js n=1000000 input='private'                                                  -0.52 %       ±3.86%  ±5.32%  ±7.29%
 http/check_invalid_header_char.js n=1000000 input='SAMEORIGIN'                                               -0.18 %       ±0.49%  ±0.68%  ±0.93%
 http/check_invalid_header_char.js n=1000000 input='Sat, 07 May 2016 16:54:48 GMT'                             0.00 %       ±0.32%  ±0.44%  ±0.61%
 http/check_invalid_header_char.js n=1000000 input='text/html; charset=utf-8'                                  0.02 %       ±0.33%  ±0.45%  ±0.62%
 http/check_invalid_header_char.js n=1000000 input='text/plain'                                               -0.24 %       ±0.64%  ±0.88%  ±1.23%
 http/check_invalid_header_char.js n=1000000 input='中文呢'                                                   -0.05 %       ±1.13%  ±1.56%  ±2.14%
 http/headers.js len=1 n=10 benchmarker='wrk'                                                                 -3.83 %      ±11.36% ±15.92% ±22.47%
 http/headers.js len=1 n=1000 benchmarker='wrk'                                                               -9.19 %      ±10.62% ±15.25% ±22.40%
 http/headers.js len=100 n=10 benchmarker='wrk'                                                               -9.16 %      ±12.37% ±17.64% ±25.66%
 http/headers.js len=100 n=1000 benchmarker='wrk'                                                             -4.43 %      ±11.55% ±16.05% ±22.38%
 http/incoming_headers.js n=0 c=50 benchmarker='wrk'                                                           3.21 %       ±9.22% ±12.64% ±17.23%
 http/incoming_headers.js n=0 c=500 benchmarker='wrk'                                                         -3.46 %       ±8.65% ±11.87% ±16.19%
 http/incoming_headers.js n=20 c=50 benchmarker='wrk'                                                         -4.85 %      ±14.23% ±19.97% ±28.26%
 http/incoming_headers.js n=20 c=500 benchmarker='wrk'                                                        -1.52 %       ±9.57% ±13.30% ±18.54%
 http/incoming_headers.js n=5 c=50 benchmarker='wrk'                                                          -1.24 %       ±9.27% ±12.76% ±17.49%
 http/incoming_headers.js n=5 c=500 benchmarker='wrk'                                                          4.18 %       ±9.91% ±13.88% ±19.62%
 http/set_header.js n=1000000 value='Connection'                                                               0.86 %       ±1.88%  ±2.59%  ±3.56%
 http/set_header.js n=1000000 value='Content-Length'                                                          -0.68 %       ±1.53%  ±2.14%  ±3.02%
 http/set_header.js n=1000000 value='Content-Type'                                                             1.56 %       ±1.74%  ±2.42%  ±3.37%
 http/set_header.js n=1000000 value='Set-Cookie'                                                              -1.10 %       ±1.84%  ±2.52%  ±3.44%
 http/set_header.js n=1000000 value='Transfer-Encoding'                                                        0.35 %       ±4.27%  ±5.88%  ±8.06%
 http/set_header.js n=1000000 value='Vary'                                                                    -2.63 %       ±4.73%  ±6.78%  ±9.94%
 http/set_header.js n=1000000 value='X-Powered-By'                                                            -0.62 %       ±1.29%  ±1.78%  ±2.44%
 http/set-header.js res='normal' benchmarker='wrk'                                                             1.24 %       ±3.05%  ±4.18%  ±5.71%
 http/set-header.js res='setHeader' benchmarker='wrk'                                                         -0.43 %       ±2.66%  ±3.64%  ±4.96%
 http/set-header.js res='setHeaderWH' benchmarker='wrk'                                                        1.35 %       ±2.69%  ±3.70%  ±5.07%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 38 comparisons, you can thus
expect the following amount of false-positive results:
  1.90 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.38 false positives, when considering a   1% risk acceptance (**, ***),
  0.04 false positives, when considering a 0.1% risk acceptance (***)

Output file (rename extension to CSV before using): compare-http-headers.txt

The command run:

node benchmark/compare.js --old ../node-master --new ../node-http-for-of --set benchmarker=wrk --filter header --runs 10 http > compare-http-headers.csv

@trivikr trivikr mentioned this pull request Dec 24, 2019
2 tasks
@trivikr
Copy link
Member Author

trivikr commented Jan 4, 2020

Comparison benchmarks of updates which weren't pushed in this PR - in files _http_client.js, _http_outgoing.js and _http_server.js

Code: https://github.com/trivikr/node/tree/http-for-of-benchmark

                                                                                               confidence improvement accuracy (*)   (**)  (***)
 http/bench-parser.js n=100000 len=16                                                                         -0.31 %       ±0.47% ±0.62% ±0.81%
 http/bench-parser.js n=100000 len=32                                                                         -0.21 %       ±0.35% ±0.47% ±0.61%
 http/bench-parser.js n=100000 len=4                                                                    *     -1.21 %       ±0.93% ±1.25% ±1.64%
 http/bench-parser.js n=100000 len=8                                                                          -0.46 %       ±1.08% ±1.43% ±1.87%
 http/check_invalid_header_char.js n=1000000 input=''                                                         -0.16 %       ±0.28% ±0.37% ±0.48%
 http/check_invalid_header_char.js n=1000000 input='\177'                                             ***     -1.45 %       ±0.56% ±0.74% ±0.97%
 http/check_invalid_header_char.js n=1000000 input='\\t\\t\\t\\t\\t\\t\\t\\t\\t\\tFoo bar baz'                -0.01 %       ±0.18% ±0.24% ±0.31%
 http/check_invalid_header_char.js n=1000000 input='1'                                                ***     -0.89 %       ±0.44% ±0.59% ±0.77%
 http/check_invalid_header_char.js n=1000000 input='20091'                                                    -0.39 %       ±0.45% ±0.60% ±0.78%
 http/check_invalid_header_char.js n=1000000 input='close'                                                    -0.08 %       ±0.28% ±0.37% ±0.48%
 http/check_invalid_header_char.js n=1000000 input='en-US'                                                    -0.13 %       ±0.28% ±0.37% ±0.48%
 http/check_invalid_header_char.js n=1000000 input='foo\\nbar'                                        ***     -1.13 %       ±0.52% ±0.69% ±0.89%
 http/check_invalid_header_char.js n=1000000 input='group_acmeair'                                            -0.04 %       ±0.16% ±0.22% ±0.28%
 http/check_invalid_header_char.js n=1000000 input='gzip'                                             ***     -0.52 %       ±0.20% ±0.27% ±0.35%
 http/check_invalid_header_char.js n=1000000 input='keep-alive'                                               -0.10 %       ±0.22% ±0.30% ±0.39%
 http/check_invalid_header_char.js n=1000000 input='LONG_AND_INVALID'                                         -0.00 %       ±0.07% ±0.09% ±0.11%
 http/check_invalid_header_char.js n=1000000 input='private'                                            *      4.18 %       ±3.49% ±4.66% ±6.10%
 http/check_invalid_header_char.js n=1000000 input='SAMEORIGIN'                                                0.07 %       ±0.21% ±0.28% ±0.37%
 http/check_invalid_header_char.js n=1000000 input='Sat, 07 May 2016 16:54:48 GMT'                            -0.32 %       ±2.66% ±3.54% ±4.60%
 http/check_invalid_header_char.js n=1000000 input='text/html; charset=utf-8'                                  2.41 %       ±2.86% ±3.80% ±4.95%
 http/check_invalid_header_char.js n=1000000 input='text/plain'                                                0.17 %       ±0.30% ±0.40% ±0.52%
 http/check_invalid_header_char.js n=1000000 input='中文呢'                                                   -0.35 %       ±0.50% ±0.66% ±0.86%
 http/check_is_http_token.js n=1000000 key=':'                                                                -0.18 %       ±0.43% ±0.57% ±0.74%
 http/check_is_http_token.js n=1000000 key=':alternate-protocol'                                               0.26 %       ±0.36% ±0.48% ±0.63%
 http/check_is_http_token.js n=1000000 key='((((())))'                                                        -0.52 %       ±0.54% ±0.71% ±0.93%
 http/check_is_http_token.js n=1000000 key='@@'                                                               -0.27 %       ±0.35% ±0.47% ±0.61%
 http/check_is_http_token.js n=1000000 key='Accept-Ranges'                                                    -0.09 %       ±0.23% ±0.30% ±0.39%
 http/check_is_http_token.js n=1000000 key='alt-svc'                                                          -0.07 %       ±0.24% ±0.32% ±0.42%
 http/check_is_http_token.js n=1000000 key='alternate-protocol:'                                              -0.07 %       ±0.23% ±0.30% ±0.39%
 http/check_is_http_token.js n=1000000 key='alternate-protocol'                                               -0.02 %       ±0.14% ±0.19% ±0.25%
 http/check_is_http_token.js n=1000000 key='Cache-Control'                                                     0.12 %       ±0.24% ±0.31% ±0.41%
 http/check_is_http_token.js n=1000000 key='Connection'                                                       -0.02 %       ±0.27% ±0.36% ±0.47%
 http/check_is_http_token.js n=1000000 key='Content-Encoding'                                                  0.10 %       ±0.18% ±0.24% ±0.31%
 http/check_is_http_token.js n=1000000 key='content-length'                                                   -0.07 %       ±0.18% ±0.24% ±0.32%
 http/check_is_http_token.js n=1000000 key='Content-Location'                                                  0.08 %       ±0.18% ±0.24% ±0.31%
 http/check_is_http_token.js n=1000000 key='content-type'                                                     -0.14 %       ±0.23% ±0.31% ±0.40%
 http/check_is_http_token.js n=1000000 key='Content-Type'                                                     -0.06 %       ±0.26% ±0.35% ±0.45%
 http/check_is_http_token.js n=1000000 key='date'                                                             -0.09 %       ±0.35% ±0.46% ±0.60%
 http/check_is_http_token.js n=1000000 key='ETag'                                                             -0.15 %       ±0.25% ±0.34% ±0.44%
 http/check_is_http_token.js n=1000000 key='Expires'                                                           0.09 %       ±0.26% ±0.35% ±0.45%
 http/check_is_http_token.js n=1000000 key='Keep-Alive'                                                        0.18 %       ±0.26% ±0.34% ±0.45%
 http/check_is_http_token.js n=1000000 key='Last-Modified'                                                    -0.17 %       ±0.24% ±0.32% ±0.41%
 http/check_is_http_token.js n=1000000 key='location'                                                          0.14 %       ±0.29% ±0.38% ±0.49%
 http/check_is_http_token.js n=1000000 key='server'                                                    **      0.31 %       ±0.23% ±0.31% ±0.40%
 http/check_is_http_token.js n=1000000 key='Server'                                                           -0.02 %       ±0.25% ±0.33% ±0.43%
 http/check_is_http_token.js n=1000000 key='status'                                                           -0.05 %       ±0.30% ±0.40% ±0.52%
 http/check_is_http_token.js n=1000000 key='TCN'                                                              -0.16 %       ±0.31% ±0.42% ±0.54%
 http/check_is_http_token.js n=1000000 key='Transfer-Encoding'                                                 0.03 %       ±0.13% ±0.17% ±0.23%
 http/check_is_http_token.js n=1000000 key='Vary'                                                              0.10 %       ±0.28% ±0.38% ±0.49%
 http/check_is_http_token.js n=1000000 key='version'                                                           0.08 %       ±0.21% ±0.29% ±0.37%
 http/check_is_http_token.js n=1000000 key='x-frame-options'                                                  -0.04 %       ±0.15% ±0.20% ±0.27%
 http/check_is_http_token.js n=1000000 key='x-xss-protection'                                                 -0.01 %       ±0.15% ±0.19% ±0.25%
 http/check_is_http_token.js n=1000000 key='中文呢'                                                           -0.06 %       ±0.41% ±0.55% ±0.72%
 http/chunked.js c=100 len=1 n=1 benchmarker='autocannon'                                                      0.00 %       ±0.15% ±0.21% ±0.27%
 http/chunked.js c=100 len=1 n=16 benchmarker='autocannon'                                                     0.02 %       ±0.16% ±0.21% ±0.28%
 http/chunked.js c=100 len=1 n=4 benchmarker='autocannon'                                                      0.01 %       ±0.10% ±0.14% ±0.18%
 http/chunked.js c=100 len=1 n=8 benchmarker='autocannon'                                                      0.01 %       ±0.08% ±0.11% ±0.15%
 http/chunked.js c=100 len=256 n=1 benchmarker='autocannon'                                            **      0.13 %       ±0.10% ±0.13% ±0.17%
 http/chunked.js c=100 len=256 n=16 benchmarker='autocannon'                                                   0.07 %       ±0.09% ±0.13% ±0.16%
 http/chunked.js c=100 len=256 n=4 benchmarker='autocannon'                                                   -0.06 %       ±0.10% ±0.13% ±0.17%
 http/chunked.js c=100 len=256 n=8 benchmarker='autocannon'                                                   -0.05 %       ±0.12% ±0.16% ±0.20%
 http/chunked.js c=100 len=64 n=1 benchmarker='autocannon'                                                     0.03 %       ±0.13% ±0.18% ±0.23%
 http/chunked.js c=100 len=64 n=16 benchmarker='autocannon'                                                    0.19 %       ±0.19% ±0.26% ±0.34%
 http/chunked.js c=100 len=64 n=4 benchmarker='autocannon'                                                    -0.05 %       ±0.10% ±0.13% ±0.17%
 http/chunked.js c=100 len=64 n=8 benchmarker='autocannon'                                                    -0.05 %       ±0.11% ±0.14% ±0.19%
 http/client-request-body.js method='end' len=1024 type='asc' dur=5                                           -0.34 %       ±3.24% ±4.31% ±5.61%
 http/client-request-body.js method='end' len=1024 type='buf' dur=5                                            2.47 %       ±3.50% ±4.66% ±6.06%
 http/client-request-body.js method='end' len=1024 type='utf' dur=5                                           -0.78 %       ±2.64% ±3.51% ±4.57%
 http/client-request-body.js method='end' len=256 type='asc' dur=5                                            -0.68 %       ±3.14% ±4.18% ±5.44%
 http/client-request-body.js method='end' len=256 type='buf' dur=5                                             2.17 %       ±2.58% ±3.45% ±4.53%
 http/client-request-body.js method='end' len=256 type='utf' dur=5                                            -0.63 %       ±3.37% ±4.48% ±5.84%
 http/client-request-body.js method='end' len=32 type='asc' dur=5                                              0.08 %       ±3.06% ±4.08% ±5.31%
 http/client-request-body.js method='end' len=32 type='buf' dur=5                                             -0.97 %       ±2.59% ±3.45% ±4.49%
 http/client-request-body.js method='end' len=32 type='utf' dur=5                                             -2.11 %       ±3.19% ±4.25% ±5.53%
 http/client-request-body.js method='write' len=1024 type='asc' dur=5                                          2.45 %       ±3.08% ±4.11% ±5.36%
 http/client-request-body.js method='write' len=1024 type='buf' dur=5                                         -0.92 %       ±3.09% ±4.11% ±5.35%
 http/client-request-body.js method='write' len=1024 type='utf' dur=5                                         -0.73 %       ±3.40% ±4.53% ±5.89%
 http/client-request-body.js method='write' len=256 type='asc' dur=5                                           1.81 %       ±3.34% ±4.45% ±5.81%
 http/client-request-body.js method='write' len=256 type='buf' dur=5                                          -1.83 %       ±3.35% ±4.46% ±5.81%
 http/client-request-body.js method='write' len=256 type='utf' dur=5                                          -1.48 %       ±3.10% ±4.12% ±5.36%
 http/client-request-body.js method='write' len=32 type='asc' dur=5                                    **     -4.37 %       ±3.09% ±4.11% ±5.36%
 http/client-request-body.js method='write' len=32 type='buf' dur=5                                            0.60 %       ±3.54% ±4.72% ±6.14%
 http/client-request-body.js method='write' len=32 type='utf' dur=5                                           -1.78 %       ±3.77% ±5.01% ±6.52%
 http/cluster.js c=50 len=1024 type='buffer' benchmarker='autocannon'                                   *     -3.01 %       ±2.59% ±3.45% ±4.49%
 http/cluster.js c=50 len=1024 type='bytes' benchmarker='autocannon'                                          -1.34 %       ±3.06% ±4.08% ±5.31%
 http/cluster.js c=50 len=102400 type='buffer' benchmarker='autocannon'                                       -0.48 %       ±1.47% ±1.96% ±2.55%
 http/cluster.js c=50 len=102400 type='bytes' benchmarker='autocannon'                                         0.54 %       ±1.65% ±2.19% ±2.86%
 http/cluster.js c=50 len=4 type='buffer' benchmarker='autocannon'                                             0.97 %       ±2.70% ±3.59% ±4.67%
 http/cluster.js c=50 len=4 type='bytes' benchmarker='autocannon'                                              0.01 %       ±2.06% ±2.73% ±3.56%
 http/cluster.js c=500 len=1024 type='buffer' benchmarker='autocannon'                                        -0.15 %       ±2.98% ±3.96% ±5.16%
 http/cluster.js c=500 len=1024 type='bytes' benchmarker='autocannon'                                         -0.10 %       ±3.17% ±4.22% ±5.49%
 http/cluster.js c=500 len=102400 type='buffer' benchmarker='autocannon'                                      -0.04 %       ±1.64% ±2.18% ±2.84%
 http/cluster.js c=500 len=102400 type='bytes' benchmarker='autocannon'                                       -0.16 %       ±1.38% ±1.84% ±2.40%
 http/cluster.js c=500 len=4 type='buffer' benchmarker='autocannon'                                            2.17 %       ±2.91% ±3.88% ±5.05%
 http/cluster.js c=500 len=4 type='bytes' benchmarker='autocannon'                                            -0.96 %       ±3.89% ±5.18% ±6.74%
 http/create-clientrequest.js e=1 arg='options' url='idn'                                                      0.45 %       ±1.37% ±1.83% ±2.38%
 http/create-clientrequest.js e=1 arg='options' url='long'                                                     0.99 %       ±1.54% ±2.05% ±2.67%
 http/create-clientrequest.js e=1 arg='options' url='wpt'                                                      0.44 %       ±1.24% ±1.65% ±2.14%
 http/create-clientrequest.js e=1 arg='string' url='idn'                                                       1.80 %       ±2.94% ±3.91% ±5.09%
 http/create-clientrequest.js e=1 arg='string' url='long'                                                      0.78 %       ±1.88% ±2.50% ±3.25%
 http/create-clientrequest.js e=1 arg='string' url='wpt'                                                *      2.30 %       ±1.96% ±2.61% ±3.40%
 http/create-clientrequest.js e=1 arg='URL' url='idn'                                                   *      1.28 %       ±1.24% ±1.65% ±2.15%
 http/create-clientrequest.js e=1 arg='URL' url='long'                                                         1.10 %       ±1.50% ±2.00% ±2.60%
 http/create-clientrequest.js e=1 arg='URL' url='wpt'                                                          0.09 %       ±1.30% ±1.73% ±2.25%
 http/end-vs-write-end.js method='end' c=100 len=1048576 type='asc' benchmarker='autocannon'                  -0.20 %       ±0.92% ±1.23% ±1.59%
 http/end-vs-write-end.js method='end' c=100 len=1048576 type='buf' benchmarker='autocannon'                  -0.49 %       ±0.84% ±1.12% ±1.46%
 http/end-vs-write-end.js method='end' c=100 len=1048576 type='utf' benchmarker='autocannon'                   0.02 %       ±0.49% ±0.66% ±0.86%
 http/end-vs-write-end.js method='end' c=100 len=131072 type='asc' benchmarker='autocannon'                   -0.83 %       ±0.96% ±1.28% ±1.67%
 http/end-vs-write-end.js method='end' c=100 len=131072 type='buf' benchmarker='autocannon'             *     -1.55 %       ±1.37% ±1.82% ±2.37%
 http/end-vs-write-end.js method='end' c=100 len=131072 type='utf' benchmarker='autocannon'                    0.48 %       ±0.88% ±1.18% ±1.53%
 http/end-vs-write-end.js method='end' c=100 len=262144 type='asc' benchmarker='autocannon'                    0.21 %       ±1.05% ±1.40% ±1.83%
 http/end-vs-write-end.js method='end' c=100 len=262144 type='buf' benchmarker='autocannon'                   -0.51 %       ±1.10% ±1.46% ±1.91%
 http/end-vs-write-end.js method='end' c=100 len=262144 type='utf' benchmarker='autocannon'                   -0.16 %       ±0.75% ±1.00% ±1.31%
 http/end-vs-write-end.js method='end' c=100 len=65536 type='asc' benchmarker='autocannon'                    -0.70 %       ±1.31% ±1.75% ±2.27%
 http/end-vs-write-end.js method='end' c=100 len=65536 type='buf' benchmarker='autocannon'                     0.71 %       ±1.53% ±2.04% ±2.65%
 http/end-vs-write-end.js method='end' c=100 len=65536 type='utf' benchmarker='autocannon'                    -0.27 %       ±1.07% ±1.42% ±1.84%
 http/end-vs-write-end.js method='write' c=100 len=1048576 type='asc' benchmarker='autocannon'                 0.90 %       ±1.43% ±1.90% ±2.47%
 http/end-vs-write-end.js method='write' c=100 len=1048576 type='buf' benchmarker='autocannon'                -0.58 %       ±0.76% ±1.01% ±1.32%
 http/end-vs-write-end.js method='write' c=100 len=1048576 type='utf' benchmarker='autocannon'                 0.53 %       ±0.74% ±0.98% ±1.28%
 http/end-vs-write-end.js method='write' c=100 len=131072 type='asc' benchmarker='autocannon'                  0.18 %       ±1.69% ±2.25% ±2.93%
 http/end-vs-write-end.js method='write' c=100 len=131072 type='buf' benchmarker='autocannon'           *      1.58 %       ±1.56% ±2.08% ±2.71%
 http/end-vs-write-end.js method='write' c=100 len=131072 type='utf' benchmarker='autocannon'                  0.21 %       ±0.83% ±1.11% ±1.45%
 http/end-vs-write-end.js method='write' c=100 len=262144 type='asc' benchmarker='autocannon'                 -0.31 %       ±1.61% ±2.15% ±2.80%
 http/end-vs-write-end.js method='write' c=100 len=262144 type='buf' benchmarker='autocannon'                 -0.92 %       ±0.99% ±1.32% ±1.72%
 http/end-vs-write-end.js method='write' c=100 len=262144 type='utf' benchmarker='autocannon'                  0.68 %       ±0.77% ±1.02% ±1.33%
 http/end-vs-write-end.js method='write' c=100 len=65536 type='asc' benchmarker='autocannon'            *      1.94 %       ±1.89% ±2.51% ±3.27%
 http/end-vs-write-end.js method='write' c=100 len=65536 type='buf' benchmarker='autocannon'                  -0.36 %       ±1.82% ±2.42% ±3.15%
 http/end-vs-write-end.js method='write' c=100 len=65536 type='utf' benchmarker='autocannon'                   0.56 %       ±1.05% ±1.39% ±1.82%
 http/headers.js len=1 n=10 benchmarker='autocannon'                                                           1.32 %       ±1.91% ±2.54% ±3.30%
 http/headers.js len=1 n=1000 benchmarker='autocannon'                                                         0.11 %       ±0.38% ±0.50% ±0.65%
 http/headers.js len=100 n=10 benchmarker='autocannon'                                                        -0.01 %       ±0.69% ±0.92% ±1.20%
 http/headers.js len=100 n=1000 benchmarker='autocannon'                                                       0.06 %       ±0.39% ±0.51% ±0.67%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 132 comparisons, you can thus
expect the following amount of false-positive results:
  6.60 false positives, when considering a   5% risk acceptance (*, **, ***),
  1.32 false positives, when considering a   1% risk acceptance (**, ***),
  0.13 false positives, when considering a 0.1% risk acceptance (***)                                                                                             

Output file (rename extension to CSV before using):
compare-http.txt

The command run:

node benchmark/compare.js --old ../node-master --new ../node-http-for-of http > compare-http.csv

@MylesBorins
Copy link
Contributor

MylesBorins commented Jan 4, 2020 via email

@Trott
Copy link
Member

Trott commented Jan 4, 2020

One thing to keep in mind here, not just with benchmarks, these types of changes do make it harder to backport to older versions of node.

I am -0 on all these changes, but would rather land them and move on than try to stop them when I don't feel that strongly. If you or someone else feels more strongly that the churn here is not worth the readability, I'd support an objection and be inclined to close-without-landing. It sounds like you're about where I am, though, around a -0.

@Trott
Copy link
Member

Trott commented Jan 4, 2020

I am -0 on all these changes,

And to answer the obvious question: My approval is "this code change works as intended and shouldn't break anything", even though I think it's basically a wash as to whether or not the cost is worth the value.

@BridgeAR BridgeAR mentioned this pull request Jan 12, 2020
2 tasks
targos pushed a commit that referenced this pull request Jan 14, 2020
PR-URL: #30958
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #30958
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
SukkaW added a commit to SukkaW/node that referenced this pull request Jul 6, 2020
Cache length prop in classic for loop for performance purpose.

Refs: nodejs#30958
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants