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.

Sign up for GitHub

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

Jump to bottom

http: cork/uncork before flushing pipelined res #3172

Closed
wants to merge 1 commit into from
Closed

http: cork/uncork before flushing pipelined res #3172

wants to merge 1 commit into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Oct 4, 2015

Make sure that the pipelined response data will be written as less TCP
packets as possible.

Benchmark results (3 runs):

http/chunked.js num=1 size=1 c=100: ./node-after: 2358.9 ./node-before: 2374.1 ................................... -0.64%                                                                           [87/709]
http/chunked.js num=1 size=64 c=100: ./node-after: 2372.7 ./node-before: 2373.2 .................................. -0.02%
http/chunked.js num=1 size=256 c=100: ./node-after: 2374.2 ./node-before: 2367.9 .................................. 0.27%
http/chunked.js num=4 size=1 c=100: ./node-after: 2363.1 ./node-before: 2356 ...................................... 0.30%
http/chunked.js num=4 size=64 c=100: ./node-after: 2370.7 ./node-before: 2358.6 ................................... 0.51%
http/chunked.js num=4 size=256 c=100: ./node-after: 2364.8 ./node-before: 2355.7 .................................. 0.39%
http/chunked.js num=8 size=1 c=100: ./node-after: 2231.5 ./node-before: 2332.9 ................................... -4.34%
http/chunked.js num=8 size=64 c=100: ./node-after: 2253.1 ./node-before: 2357.3 .................................. -4.42%
http/chunked.js num=8 size=256 c=100: ./node-after: 2346.7 ./node-before: 2300.2 .................................. 2.02%
http/chunked.js num=16 size=1 c=100: ./node-after: 2248.2 ./node-before: 2076.3 ................................... 8.28%
http/chunked.js num=16 size=64 c=100: ./node-after: 2223.2 ./node-before: 2052.4 .................................. 8.32%
http/chunked.js num=16 size=256 c=100: ./node-after: 1928.6 ./node-before: 2133 .................................. -9.58%
http/client-request-body.js dur=5 type=asc bytes=32 method=write: ./node-after: 3296.9 ./node-before: 49.987 ... 6495.58%
http/client-request-body.js dur=5 type=asc bytes=32 method=end: ./node-after: 3308.6 ./node-before: 49.598 ..... 6570.96%
http/client-request-body.js dur=5 type=asc bytes=256 method=write: ./node-after: 3462.5 ./node-before: 49.595 .. 6881.48%
http/client-request-body.js dur=5 type=asc bytes=256 method=end: ./node-after: 3520 ./node-before: 49.599 ...... 6997.02%
http/client-request-body.js dur=5 type=asc bytes=1024 method=write: ./node-after: 3384.3 ./node-before: 49.59 .. 6724.59%
http/client-request-body.js dur=5 type=asc bytes=1024 method=end: ./node-after: 3409.9 ./node-before: 49.594 ... 6775.71%
http/client-request-body.js dur=5 type=utf bytes=32 method=write: ./node-after: 3373.4 ./node-before: 49.597 ... 6701.50%
http/client-request-body.js dur=5 type=utf bytes=32 method=end: ./node-after: 3319.9 ./node-before: 49.991 ..... 6540.99%
http/client-request-body.js dur=5 type=utf bytes=256 method=write: ./node-after: 3606.5 ./node-before: 49.994 .. 7113.99%
http/client-request-body.js dur=5 type=utf bytes=256 method=end: ./node-after: 3211 ./node-before: 49.592 ...... 6374.84%
http/client-request-body.js dur=5 type=utf bytes=1024 method=write: ./node-after: 2963.8 ./node-before: 49.585 . 5877.28%
http/client-request-body.js dur=5 type=utf bytes=1024 method=end: ./node-after: 3298.2 ./node-before: 49.986 ... 6498.27%
http/client-request-body.js dur=5 type=buf bytes=32 method=write: ./node-after: 3454.9 ./node-before: 49.99 .... 6811.27%
http/client-request-body.js dur=5 type=buf bytes=32 method=end: ./node-after: 3512.3 ./node-before: 49.599 ..... 6981.38%
http/client-request-body.js dur=5 type=buf bytes=256 method=write: ./node-after: 3459.6 ./node-before: 49.595 .. 6875.69%
http/client-request-body.js dur=5 type=buf bytes=256 method=end: ./node-after: 3642.6 ./node-before: 49.99 ..... 7186.51%
http/client-request-body.js dur=5 type=buf bytes=1024 method=write: ./node-after: 3522.2 ./node-before: 49.989 . 6945.95%
http/client-request-body.js dur=5 type=buf bytes=1024 method=end: ./node-after: 3551 ./node-before: 49.597 ..... 7059.66%
http/cluster.js type=bytes length=4 c=50: ./node-after: 17203 ./node-before: 14735 ............................... 16.75%
http/cluster.js type=bytes length=4 c=500: ./node-after: 15383 ./node-before: 15141 ............................... 1.60%
http/cluster.js type=bytes length=1024 c=50: ./node-after: 15554 ./node-before: 15133 ............................. 2.78%
http/cluster.js type=bytes length=1024 c=500: ./node-after: 14598 ./node-before: 14316 ............................ 1.97%
http/cluster.js type=bytes length=102400 c=50: ./node-after: 844.36 ./node-before: 887.37 ........................ -4.85%
http/cluster.js type=bytes length=102400 c=500: ./node-after: 760.55 ./node-before: 850.88 ...................... -10.62%
http/cluster.js type=buffer length=4 c=50: ./node-after: 14737 ./node-before: 15454 .............................. -4.64%
http/cluster.js type=buffer length=4 c=500: ./node-after: 13455 ./node-before: 13824 ............................. -2.67%
http/cluster.js type=buffer length=1024 c=50: ./node-after: 14033 ./node-before: 14390 ........................... -2.48%
http/cluster.js type=buffer length=1024 c=500: ./node-after: 14124 ./node-before: 14843 .......................... -4.85%
http/cluster.js type=buffer length=102400 c=50: ./node-after: 12275 ./node-before: 11311 .......................... 8.52%
http/cluster.js type=buffer length=102400 c=500: ./node-after: 11404 ./node-before: 12162 ........................ -6.23%
http/end-vs-write-end.js type=asc kb=64 c=100 method=write: ./node-after: 4567.7 ./node-before: 4516.2 ............ 1.14%
http/end-vs-write-end.js type=asc kb=64 c=100 method=end: ./node-after: 4582.2 ./node-before: 4417.7 .............. 3.72%
http/end-vs-write-end.js type=asc kb=128 c=100 method=write: ./node-after: 2688.4 ./node-before: 2932.7 .......... -8.33%
http/end-vs-write-end.js type=asc kb=128 c=100 method=end: ./node-after: 2985.8 ./node-before: 3066.6 ............ -2.63%
http/end-vs-write-end.js type=asc kb=256 c=100 method=write: ./node-after: 1733.4 ./node-before: 1703.8 ........... 1.74%
http/end-vs-write-end.js type=asc kb=256 c=100 method=end: ./node-after: 1805.5 ./node-before: 1656.4 ............. 9.00%
http/end-vs-write-end.js type=asc kb=1024 c=100 method=write: ./node-after: 361.9 ./node-before: 362.04 .......... -0.04%
http/end-vs-write-end.js type=asc kb=1024 c=100 method=end: ./node-after: 362.07 ./node-before: 357.1 ............. 1.39%
http/end-vs-write-end.js type=utf kb=64 c=100 method=write: ./node-after: 3483.3 ./node-before: 3697.7 ........... -5.80%
http/end-vs-write-end.js type=utf kb=64 c=100 method=end: ./node-after: 3986.4 ./node-before: 3818 ................ 4.41%
http/end-vs-write-end.js type=utf kb=128 c=100 method=write: ./node-after: 2320.3 ./node-before: 2177.1 ........... 6.58%
http/end-vs-write-end.js type=utf kb=128 c=100 method=end: ./node-after: 2532.2 ./node-before: 2437.6 ............. 3.88%
http/end-vs-write-end.js type=utf kb=256 c=100 method=write: ./node-after: 1388 ./node-before: 1411.2 ............ -1.65%
http/end-vs-write-end.js type=utf kb=256 c=100 method=end: ./node-after: 1622.8 ./node-before: 1367.6 ............ 18.67%
http/end-vs-write-end.js type=utf kb=1024 c=100 method=write: ./node-after: 412.43 ./node-before: 415.73 ......... -0.79%
http/end-vs-write-end.js type=utf kb=1024 c=100 method=end: ./node-after: 385.78 ./node-before: 399.51 ........... -3.44%
http/end-vs-write-end.js type=buf kb=64 c=100 method=write: ./node-after: 7876.5 ./node-before: 6939.5 ........... 13.50%
http/end-vs-write-end.js type=buf kb=64 c=100 method=end: ./node-after: 7916.3 ./node-before: 7418.2 .............. 6.71%
http/end-vs-write-end.js type=buf kb=128 c=100 method=write: ./node-after: 6000.9 ./node-before: 6474.2 .......... -7.31%
http/end-vs-write-end.js type=buf kb=128 c=100 method=end: ./node-after: 6715.9 ./node-before: 6786.8 ............ -1.04%
http/end-vs-write-end.js type=buf kb=256 c=100 method=write: ./node-after: 5026.9 ./node-before: 5400.1 .......... -6.91%
http/end-vs-write-end.js type=buf kb=256 c=100 method=end: ./node-after: 5380.5 ./node-before: 5861.6 ............ -8.21%
http/end-vs-write-end.js type=buf kb=1024 c=100 method=write: ./node-after: 2646.3 ./node-before: 2466.3 .......... 7.30%
http/end-vs-write-end.js type=buf kb=1024 c=100 method=end: ./node-after: 2518.6 ./node-before: 2407.1 ............ 4.63%
http/_chunky_http_client.js len=1 num=5 type=send: ./node-after: 64.799 ./node-before: 64.931 .................... -0.20%
http/_chunky_http_client.js len=1 num=50 type=send: ./node-after: 173.88 ./node-before: 187.61 ................... -7.32%
http/_chunky_http_client.js len=1 num=500 type=send: ./node-after: 243.19 ./node-before: 233.04 ................... 4.36%
http/_chunky_http_client.js len=1 num=2000 type=send: ./node-after: 239.9 ./node-before: 247.48 .................. -3.06%
http/_chunky_http_client.js len=4 num=5 type=send: ./node-after: 135.87 ./node-before: 161.15 ................... -15.69%
http/_chunky_http_client.js len=4 num=50 type=send: ./node-after: 176.15 ./node-before: 131.64 ................... 33.81%
http/_chunky_http_client.js len=4 num=500 type=send: ./node-after: 219.26 ./node-before: 237.87 .................. -7.82%
http/_chunky_http_client.js len=4 num=2000 type=send: ./node-after: 271.31 ./node-before: 224.63 ................. 20.78%
http/_chunky_http_client.js len=8 num=5 type=send: ./node-after: 120.67 ./node-before: 138.4 .................... -12.81%
http/_chunky_http_client.js len=8 num=50 type=send: ./node-after: 187.72 ./node-before: 202.46 ................... -7.28%
http/_chunky_http_client.js len=8 num=500 type=send: ./node-after: 222.44 ./node-before: 219.25 ................... 1.45%
http/_chunky_http_client.js len=8 num=2000 type=send: ./node-after: 275.45 ./node-before: 208.39 ................. 32.18%
http/_chunky_http_client.js len=16 num=5 type=send: ./node-after: 128.75 ./node-before: 114.76 ................... 12.19%
http/_chunky_http_client.js len=16 num=50 type=send: ./node-after: 169.7 ./node-before: 199.12 .................. -14.77%
http/_chunky_http_client.js len=16 num=500 type=send: ./node-after: 203.63 ./node-before: 208.51 ................. -2.34%
http/_chunky_http_client.js len=16 num=2000 type=send: ./node-after: 244.63 ./node-before: 235.49 ................. 3.88%
http/_chunky_http_client.js len=32 num=5 type=send: ./node-after: 104.66 ./node-before: 48.936 .................. 113.87%
http/_chunky_http_client.js len=32 num=50 type=send: ./node-after: 202.27 ./node-before: 162.8 ................... 24.25%
http/_chunky_http_client.js len=32 num=500 type=send: ./node-after: 242.88 ./node-before: 241.29 .................. 0.66%
http/_chunky_http_client.js len=32 num=2000 type=send: ./node-after: 239.01 ./node-before: 206.21 ................ 15.91%
http/_chunky_http_client.js len=64 num=5 type=send: ./node-after: 143.12 ./node-before: 48.008 .................. 198.12%
http/_chunky_http_client.js len=64 num=50 type=send: ./node-after: 222.88 ./node-before: 135.39 .................. 64.62%
http/_chunky_http_client.js len=64 num=500 type=send: ./node-after: 227.41 ./node-before: 240.4 .................. -5.40%
http/_chunky_http_client.js len=64 num=2000 type=send: ./node-after: 264.16 ./node-before: 251.04 ................. 5.23%
http/_chunky_http_client.js len=128 num=5 type=send: ./node-after: 46.779 ./node-before: 44.096 ................... 6.09%
http/_chunky_http_client.js len=128 num=50 type=send: ./node-after: 61.551 ./node-before: 188.66 ................ -67.37%
http/_chunky_http_client.js len=128 num=500 type=send: ./node-after: 217.13 ./node-before: 266.04 ............... -18.38%
http/_chunky_http_client.js len=128 num=2000 type=send: ./node-after: 232.96 ./node-before: 259.63 .............. -10.28%
http/simple.js type=bytes length=4 chunks=0 c=50: ./node-after: 9245.3 ./node-before: 9118.4 ...................... 1.39%
http/simple.js type=bytes length=4 chunks=0 c=500: ./node-after: 9255.5 ./node-before: 9152.4 ..................... 1.13%
http/simple.js type=bytes length=4 chunks=1 c=50: ./node-after: 8326.3 ./node-before: 8520.3 ..................... -2.28%
http/simple.js type=bytes length=4 chunks=1 c=500: ./node-after: 7902.2 ./node-before: 7738.2 ..................... 2.12%
http/simple.js type=bytes length=4 chunks=4 c=50: ./node-after: 1185.1 ./node-before: 1189.9 ..................... -0.40%
http/simple.js type=bytes length=4 chunks=4 c=500: ./node-after: 5658.5 ./node-before: 5339.8 ..................... 5.97%
http/simple.js type=bytes length=1024 chunks=0 c=50: ./node-after: 8493 ./node-before: 8849.1 .................... -4.03%
http/simple.js type=bytes length=1024 chunks=0 c=500: ./node-after: 8121.5 ./node-before: 8035 .................... 1.08%
http/simple.js type=bytes length=1024 chunks=1 c=50: ./node-after: 6813.1 ./node-before: 6005.2 .................. 13.45%
http/simple.js type=bytes length=1024 chunks=1 c=500: ./node-after: 6371.4 ./node-before: 7003.9 ................. -9.03%
http/simple.js type=bytes length=1024 chunks=4 c=50: ./node-after: 1189.9 ./node-before: 1191.7 .................. -0.15%
http/simple.js type=bytes length=1024 chunks=4 c=500: ./node-after: 4826.9 ./node-before: 5148.8 ................. -6.25%
http/simple.js type=bytes length=102400 chunks=0 c=50: ./node-after: 397.49 ./node-before: 435.99 ................ -8.83%
http/simple.js type=bytes length=102400 chunks=0 c=500: ./node-after: 410.8 ./node-before: 445.48 ................ -7.78%
http/simple.js type=bytes length=102400 chunks=1 c=50: ./node-after: 235.8 ./node-before: 251.15 ................. -6.11%
http/simple.js type=bytes length=102400 chunks=1 c=500: ./node-after: 254.27 ./node-before: 253.64 ................ 0.25%
http/simple.js type=bytes length=102400 chunks=4 c=50: ./node-after: 1953.3 ./node-before: 2025.7 ................ -3.57%
http/simple.js type=bytes length=102400 chunks=4 c=500: ./node-after: 1822.2 ./node-before: 1971.7 ............... -7.58%
http/simple.js type=buffer length=4 chunks=0 c=50: ./node-after: 7697.4 ./node-before: 8308.6 .................... -7.36%
http/simple.js type=buffer length=4 chunks=0 c=500: ./node-after: 7722 ./node-before: 7645.4 ...................... 1.00%
http/simple.js type=buffer length=4 chunks=1 c=50: ./node-after: 7423 ./node-before: 7111.4 ....................... 4.38%
http/simple.js type=buffer length=4 chunks=1 c=500: ./node-after: 7565.9 ./node-before: 7081.1 .................... 6.85%
http/simple.js type=buffer length=4 chunks=4 c=50: ./node-after: 7343 ./node-before: 6543.7 ...................... 12.21%
http/simple.js type=buffer length=4 chunks=4 c=500: ./node-after: 6732.3 ./node-before: 6857.2 ................... -1.82%
http/simple.js type=buffer length=1024 chunks=0 c=50: ./node-after: 8176.5 ./node-before: 8441.2 ................. -3.14%
http/simple.js type=buffer length=1024 chunks=0 c=500: ./node-after: 7648.5 ./node-before: 8213.4 ................ -6.88%
http/simple.js type=buffer length=1024 chunks=1 c=50: ./node-after: 8139.4 ./node-before: 7969 .................... 2.14%
http/simple.js type=buffer length=1024 chunks=1 c=500: ./node-after: 7342.6 ./node-before: 7223.7 ................. 1.65%
http/simple.js type=buffer length=1024 chunks=4 c=50: ./node-after: 6663.5 ./node-before: 7265.7 ................. -8.29%
http/simple.js type=buffer length=1024 chunks=4 c=500: ./node-after: 5862.8 ./node-before: 6467.6 ................ -9.35%
http/simple.js type=buffer length=102400 chunks=0 c=50: ./node-after: 6431.9 ./node-before: 6522.8 ............... -1.39%
http/simple.js type=buffer length=102400 chunks=0 c=500: ./node-after: 6000.1 ./node-before: 6609.7 .............. -9.22%
http/simple.js type=buffer length=102400 chunks=1 c=50: ./node-after: 6263.9 ./node-before: 6122.4 ................ 2.31%
http/simple.js type=buffer length=102400 chunks=1 c=500: ./node-after: 6340.6 ./node-before: 5743 ................ 10.40%
http/simple.js type=buffer length=102400 chunks=4 c=50: ./node-after: 5644.4 ./node-before: 5665.6 ............... -0.37%
http/simple.js type=buffer length=102400 chunks=4 c=500: ./node-after: 5786.2 ./node-before: 5589.8 ............... 3.51%

cc @trevnorris (we need performance WG)

@indutny
http: cork/uncork before flushing pipelined res
51dc64b 
Make sure that the pipelined response data will be written as less TCP
packets as possible.
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Oct 4, 2015
@mscdex
Copy link
Contributor

mscdex commented Oct 4, 2015

LGTM

@indutny
Copy link
Member Author

indutny commented Oct 4, 2015

May I ask anybody to re-run the benchmarks to confirm that these -60% a just spurious?

@trevnorris
Copy link
Contributor

@indutny The http benchmarks vary a lot. That difference appears to just be some test variance.

OutgoingMessage#write() is already calling cork() in the case where data is a buffer. I remember trying to generalize it to all cases but for some reason had an issue with that. My worry is that you'll be uncorking on a connection that's still corked if a user writes both buffers and strings. Think it could be generalized to all cases?

The other difference is that write() calls uncork() on the nextTick() instead of when the data is done being written. (this may only affect the case of _writeRaw()).

@indutny
Copy link
Member Author

indutny commented Oct 6, 2015

@trevnorris cork calls are balanced, no spurious uncorking will happen...

@trevnorris
Copy link
Contributor

Cool. Then LGTM.

@indutny
Copy link
Member Author

indutny commented Oct 6, 2015

CI: https://ci.nodejs.org/job/node-test-pull-request/436/

@trevnorris
Copy link
Contributor

Tests look good. Only external string bytes is failing.

@indutny
Copy link
Member Author

indutny commented Oct 6, 2015

CI is green

@indutny
Copy link
Member Author

indutny commented Oct 6, 2015

Landed in 184f09d, thank you everyone!

@indutny indutny closed this Oct 6, 2015
@indutny indutny deleted the feature/faster-pipelining branch October 6, 2015 23:42
indutny added a commit that referenced this pull request Oct 6, 2015
@indutny
http: cork/uncork before flushing pipelined res
184f09d 
Make sure that the pipelined response data will be written as less TCP
packets as possible.

PR-URL: #3172
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@jasnell jasnell mentioned this pull request Oct 8, 2015
Closed
29 tasks
indutny added a commit that referenced this pull request Oct 8, 2015
@indutny @jasnell
http: cork/uncork before flushing pipelined res
2ddbbfd 
Make sure that the pipelined response data will be written as less TCP
packets as possible.

PR-URL: #3172
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@jasnell jasnell mentioned this pull request Oct 8, 2015
Closed
@aliscco aliscco mentioned this pull request Jun 25, 2023
Open
@aliscco aliscco mentioned this pull request Nov 29, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@indutny @mscdex @trevnorris @jasnell