Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Feature/writev master #5257

Merged
merged 5 commits into from Apr 28, 2013
Merged

Feature/writev master #5257

merged 5 commits into from Apr 28, 2013

Conversation

indutny
Copy link
Member

@indutny indutny commented Apr 8, 2013

/cc @isaacs @bnoordhuis

Previous discussion: #5246

@indutny
Copy link
Member Author

indutny commented Apr 9, 2013

So its like "mostly faster" on http benchmarks.

@indutny
Copy link
Member Author

indutny commented Apr 9, 2013

Updated pull request to be more consistent with newly added coalesce API for streams2. The latests commits are introducing tcp.writev method which can accept mixed array of buffers and strings with encodings, under the hood it copies them to one storage and invoke uv_write() only once. Benchmark results are coming soon...

@indutny
Copy link
Member Author

indutny commented Apr 9, 2013

Removed all benchmark results, just realized that I was running them on CI server.

@indutny
Copy link
Member Author

indutny commented Apr 9, 2013

Benchmark results (sorry difference is inversed):

http/chunked.js num=1 size=1 c=100: ./node-original: 1350.7 ./node: 1635.6 ........................... -17.42%
http/chunked.js num=1 size=64 c=100: ./node-original: 1338.4 ./node: 1617.9 .......................... -17.27%
http/chunked.js num=1 size=256 c=100: ./node-original: 1334.5 ./node: 1656.6 ......................... -19.44%
http/chunked.js num=4 size=1 c=100: ./node-original: 463.64 ./node: 754.69 ........................... -38.56%
http/chunked.js num=4 size=64 c=100: ./node-original: 472.62 ./node: 734.34 .......................... -35.64%
http/chunked.js num=4 size=256 c=100: ./node-original: 453 ./node: 725.06 ............................ -37.52%
http/chunked.js num=8 size=1 c=100: ./node-original: 247 ./node: 417.25 .............................. -40.80%
http/chunked.js num=8 size=64 c=100: ./node-original: 243.37 ./node: 415.04 .......................... -41.36%
http/chunked.js num=8 size=256 c=100: ./node-original: 238.49 ./node: 413.63 ......................... -42.34%
http/chunked.js num=16 size=1 c=100: ./node-original: 121.18 ./node: 229.34 .......................... -47.16%
http/chunked.js num=16 size=64 c=100: ./node-original: 123.02 ./node: 225.3 .......................... -45.40%
http/chunked.js num=16 size=256 c=100: ./node-original: 122 ./node: 227.8 ............................ -46.44%
http/cluster.js type=bytes length=4 c=50: ./node-original: 3285 ./node: 3194.8 ......................... 2.82%
http/cluster.js type=bytes length=4 c=500: ./node-original: 3200.1 ./node: 3168.4 ...................... 1.00%
http/cluster.js type=bytes length=1024 c=50: ./node-original: 3203 ./node: 3254 ....................... -1.57%
http/cluster.js type=bytes length=1024 c=500: ./node-original: 3178.3 ./node: 3107.4 ................... 2.28%
http/cluster.js type=bytes length=102400 c=50: ./node-original: 471.93 ./node: 470.87 .................. 0.23%
http/cluster.js type=bytes length=102400 c=500: ./node-original: 487.8 ./node: 487.2 ................... 0.12%
http/cluster.js type=buffer length=4 c=50: ./node-original: 3312.9 ./node: 3130.9 ...................... 5.81%
http/cluster.js type=buffer length=4 c=500: ./node-original: 3094.8 ./node: 3023.1 ..................... 2.37%
http/cluster.js type=buffer length=1024 c=50: ./node-original: 3096.6 ./node: 3295.8 .................. -6.04%
http/cluster.js type=buffer length=1024 c=500: ./node-original: 3107.8 ./node: 3010 .................... 3.25%
http/cluster.js type=buffer length=102400 c=50: ./node-original: 1694.8 ./node: 1681.3 ................. 0.81%
http/cluster.js type=buffer length=102400 c=500: ./node-original: 1877.5 ./node: 1792.1 ................ 4.77%
http/end-vs-write-end.js type=asc kb=64 c=100 method=write: ./node-original: 938.68 ./node: 1077.3 ... -12.87%
http/end-vs-write-end.js type=asc kb=64 c=100 method=end  : ./node-original: 1096 ./node: 1154.9 ...... -5.09%
http/end-vs-write-end.js type=asc kb=128 c=100 method=write: ./node-original: 656.37 ./node: 701.15 ... -6.39%
http/end-vs-write-end.js type=asc kb=128 c=100 method=end  : ./node-original: 716.93 ./node: 712.99 .... 0.55%
http/end-vs-write-end.js type=asc kb=256 c=100 method=write: ./node-original: 403.77 ./node: 368.3 ..... 9.63%
http/end-vs-write-end.js type=asc kb=256 c=100 method=end  : ./node-original: 420.78 ./node: 416.51 .... 1.03%
http/end-vs-write-end.js type=asc kb=1024 c=100 method=write: ./node-original: 103.39 ./node: 115.53 . -10.51%
http/end-vs-write-end.js type=asc kb=1024 c=100 method=end  : ./node-original: 101.72 ./node: 104.2 ... -2.38%
http/end-vs-write-end.js type=utf kb=64 c=100 method=write: ./node-original: 1117.3 ./node: 1286.8 ... -13.18%
http/end-vs-write-end.js type=utf kb=64 c=100 method=end  : ./node-original: 1330.9 ./node: 1360.6 .... -2.18%
http/end-vs-write-end.js type=utf kb=128 c=100 method=write: ./node-original: 739.31 ./node: 810.58 ... -8.79%
http/end-vs-write-end.js type=utf kb=128 c=100 method=end  : ./node-original: 824.67 ./node: 840.18 ... -1.85%
http/end-vs-write-end.js type=utf kb=256 c=100 method=write: ./node-original: 492.41 ./node: 531.61 ... -7.37%
http/end-vs-write-end.js type=utf kb=256 c=100 method=end  : ./node-original: 535.62 ./node: 508.7 ..... 5.29%
http/end-vs-write-end.js type=utf kb=1024 c=100 method=write: ./node-original: 163.93 ./node: 144.73 .. 13.26%
http/end-vs-write-end.js type=utf kb=1024 c=100 method=end  : ./node-original: 162.43 ./node: 162.9 ... -0.29%
http/end-vs-write-end.js type=buf kb=64 c=100 method=write: ./node-original: 1043.3 ./node: 1677.5 ... -37.81%
http/end-vs-write-end.js type=buf kb=64 c=100 method=end  : ./node-original: 1681.6 ./node: 1607.3 ..... 4.62%
http/end-vs-write-end.js type=buf kb=128 c=100 method=write: ./node-original: 872.07 ./node: 1481.3 .. -41.13%
http/end-vs-write-end.js type=buf kb=128 c=100 method=end  : ./node-original: 880.55 ./node: 1363.6 .. -35.42%
http/end-vs-write-end.js type=buf kb=256 c=100 method=write: ./node-original: 723.88 ./node: 1106.5 .. -34.58%
http/end-vs-write-end.js type=buf kb=256 c=100 method=end  : ./node-original: 746.71 ./node: 1111.7 .. -32.83%
http/end-vs-write-end.js type=buf kb=1024 c=100 method=write: ./node-original: 368.01 ./node: 492.63 . -25.30%
http/end-vs-write-end.js type=buf kb=1024 c=100 method=end  : ./node-original: 368.08 ./node: 509.32 . -27.73%
http/simple.js type=bytes length=4 c=50: ./node-original: 2204.9 ./node: 2268.5 ....................... -2.80%
http/simple.js type=bytes length=4 c=500: ./node-original: 2083.3 ./node: 2010 ......................... 3.64%
http/simple.js type=bytes length=1024 c=50: ./node-original: 2198.5 ./node: 2350.3 .................... -6.46%
http/simple.js type=bytes length=1024 c=500: ./node-original: 2008.7 ./node: 1987 ...................... 1.10%
http/simple.js type=bytes length=102400 c=50: ./node-original: 332.78 ./node: 328.77 ................... 1.22%
http/simple.js type=bytes length=102400 c=500: ./node-original: 316.08 ./node: 304.85 .................. 3.68%
http/simple.js type=buffer length=4 c=50: ./node-original: 2199.9 ./node: 2113.9 ....................... 4.07%
http/simple.js type=buffer length=4 c=500: ./node-original: 2034.8 ./node: 1999.5 ...................... 1.76%
http/simple.js type=buffer length=1024 c=50: ./node-original: 2177.1 ./node: 2168.3 .................... 0.40%
http/simple.js type=buffer length=1024 c=500: ./node-original: 2078.2 ./node: 1975.5 ................... 5.20%
http/simple.js type=buffer length=102400 c=50: ./node-original: 1265.9 ./node: 1255.6 .................. 0.82%
http/simple.js type=buffer length=102400 c=500: ./node-original: 1223.5 ./node: 1230.3 ................ -0.56%

@indutny
Copy link
Member Author

indutny commented Apr 14, 2013

@bnoordhuis @isaacs this coalesce thing seems to be to broad and I agree with @bnoordhuis that using it for every net write is a bad idea. I've force pushed this PR and now it implements bulk method for writable stream and consumes it in http module. Please let me know how it looks for you!

@indutny
Copy link
Member Author

indutny commented Apr 14, 2013

Fresh benchmark results:


http/chunked.js num=1 size=1 c=100: ./node-with-changes: 1709.2 ./node-original: 1325.6 ........................... 28.94%
http/chunked.js num=1 size=64 c=100: ./node-with-changes: 1645.6 ./node-original: 1304.3 .......................... 26.17%
http/chunked.js num=1 size=256 c=100: ./node-with-changes: 1666.8 ./node-original: 1292.9 ......................... 28.92%
http/chunked.js num=4 size=1 c=100: ./node-with-changes: 736.32 ./node-original: 452.2 ............................ 62.83%
http/chunked.js num=4 size=64 c=100: ./node-with-changes: 765.07 ./node-original: 452.06 .......................... 69.24%
http/chunked.js num=4 size=256 c=100: ./node-with-changes: 723.93 ./node-original: 448.04 ......................... 61.58%
http/chunked.js num=8 size=1 c=100: ./node-with-changes: 407.65 ./node-original: 243.07 ........................... 67.71%
http/chunked.js num=8 size=64 c=100: ./node-with-changes: 413.22 ./node-original: 240.91 .......................... 71.53%
http/chunked.js num=8 size=256 c=100: ./node-with-changes: 419.79 ./node-original: 230.4 .......................... 82.20%
http/chunked.js num=16 size=1 c=100: ./node-with-changes: 229.7 ./node-original: 122.28 ........................... 87.85%
http/chunked.js num=16 size=64 c=100: ./node-with-changes: 229.65 ./node-original: 122.25 ......................... 87.85%
http/chunked.js num=16 size=256 c=100: ./node-with-changes: 224.34 ./node-original: 118.68 ........................ 89.03%
http/cluster.js type=bytes length=4 c=50: ./node-with-changes: 3157.1 ./node-original: 3159.2 ..................... -0.06%
http/cluster.js type=bytes length=4 c=500: ./node-with-changes: 3191.3 ./node-original: 3203.4 .................... -0.38%
http/cluster.js type=bytes length=1024 c=50: ./node-with-changes: 3353.4 ./node-original: 3106.8 ................... 7.94%
http/cluster.js type=bytes length=1024 c=500: ./node-with-changes: 3076 ./node-original: 3081.9 ................... -0.19%
http/cluster.js type=bytes length=102400 c=50: ./node-with-changes: 438.89 ./node-original: 439.98 ................ -0.25%
http/cluster.js type=bytes length=102400 c=500: ./node-with-changes: 450.17 ./node-original: 463.46 ............... -2.87%
http/cluster.js type=buffer length=4 c=50: ./node-with-changes: 3375.9 ./node-original: 3244.9 ..................... 4.04%
http/cluster.js type=buffer length=4 c=500: ./node-with-changes: 3053.9 ./node-original: 3172.2 ................... -3.73%
http/cluster.js type=buffer length=1024 c=50: ./node-with-changes: 3295.4 ./node-original: 3229.4 .................. 2.05%
http/cluster.js type=buffer length=1024 c=500: ./node-with-changes: 2930.4 ./node-original: 2954.9 ................ -0.83%
http/cluster.js type=buffer length=102400 c=50: ./node-with-changes: 1708.3 ./node-original: 1672.2 ................ 2.16%
http/cluster.js type=buffer length=102400 c=500: ./node-with-changes: 1837.9 ./node-original: 1855.8 .............. -0.96%
http/end-vs-write-end.js type=asc kb=64 c=100 method=write: ./node-with-changes: 914.03 ./node-original: 939.64 ... -2.73%
http/end-vs-write-end.js type=asc kb=64 c=100 method=end  : ./node-with-changes: 1063.7 ./node-original: 1078 ..... -1.32%
http/end-vs-write-end.js type=asc kb=128 c=100 method=write: ./node-with-changes: 628.76 ./node-original: 627.02 ... 0.28%
http/end-vs-write-end.js type=asc kb=128 c=100 method=end  : ./node-with-changes: 683.45 ./node-original: 679.46 ... 0.59%
http/end-vs-write-end.js type=asc kb=256 c=100 method=write: ./node-with-changes: 386.71 ./node-original: 391.43 .. -1.21%
http/end-vs-write-end.js type=asc kb=256 c=100 method=end  : ./node-with-changes: 409.83 ./node-original: 406.41 ... 0.84%
http/end-vs-write-end.js type=asc kb=1024 c=100 method=write: ./node-with-changes: 101.64 ./node-original: 102.08 . -0.43%
http/end-vs-write-end.js type=asc kb=1024 c=100 method=end  : ./node-with-changes: 103.6 ./node-original: 99.957 ... 3.65%
http/end-vs-write-end.js type=utf kb=64 c=100 method=write: ./node-with-changes: 1067.8 ./node-original: 1085.4 ... -1.63%
http/end-vs-write-end.js type=utf kb=64 c=100 method=end  : ./node-with-changes: 1280.5 ./node-original: 1339 ..... -4.38%
http/end-vs-write-end.js type=utf kb=128 c=100 method=write: ./node-with-changes: 765.84 ./node-original: 746.59 ... 2.58%
http/end-vs-write-end.js type=utf kb=128 c=100 method=end  : ./node-with-changes: 818.96 ./node-original: 837.74 .. -2.24%
http/end-vs-write-end.js type=utf kb=256 c=100 method=write: ./node-with-changes: 492.8 ./node-original: 492.54 .... 0.05%
http/end-vs-write-end.js type=utf kb=256 c=100 method=end  : ./node-with-changes: 530.3 ./node-original: 528.3 ..... 0.38%
http/end-vs-write-end.js type=utf kb=1024 c=100 method=write: ./node-with-changes: 155.13 ./node-original: 156.87 . -1.11%
http/end-vs-write-end.js type=utf kb=1024 c=100 method=end  : ./node-with-changes: 159.63 ./node-original: 158.48 .. 0.73%
http/end-vs-write-end.js type=buf kb=64 c=100 method=write: ./node-with-changes: 1297.5 ./node-original: 1009.6 ... 28.52%
http/end-vs-write-end.js type=buf kb=64 c=100 method=end  : ./node-with-changes: 1637.3 ./node-original: 1691 ..... -3.17%
http/end-vs-write-end.js type=buf kb=128 c=100 method=write: ./node-with-changes: 1112.5 ./node-original: 861.66 .. 29.11%
http/end-vs-write-end.js type=buf kb=128 c=100 method=end  : ./node-with-changes: 1111 ./node-original: 868.84 .... 27.88%
http/end-vs-write-end.js type=buf kb=256 c=100 method=write: ./node-with-changes: 944.22 ./node-original: 738.03 .. 27.94%
http/end-vs-write-end.js type=buf kb=256 c=100 method=end  : ./node-with-changes: 930 ./node-original: 730.26 ..... 27.35%
http/end-vs-write-end.js type=buf kb=1024 c=100 method=write: ./node-with-changes: 440.38 ./node-original: 346.51 . 27.09%
http/end-vs-write-end.js type=buf kb=1024 c=100 method=end  : ./node-with-changes: 445.63 ./node-original: 349.54 . 27.49%
http/simple.js type=bytes length=4 c=50: ./node-with-changes: 2189.8 ./node-original: 2335.6 ...................... -6.24%
http/simple.js type=bytes length=4 c=500: ./node-with-changes: 2165.4 ./node-original: 2126.9 ...................... 1.81%
http/simple.js type=bytes length=1024 c=50: ./node-with-changes: 2247.2 ./node-original: 2160.6 .................... 4.01%
http/simple.js type=bytes length=1024 c=500: ./node-with-changes: 1978 ./node-original: 2059.4 .................... -3.95%
http/simple.js type=bytes length=102400 c=50: ./node-with-changes: 310.34 ./node-original: 310.36 ................. -0.01%
http/simple.js type=bytes length=102400 c=500: ./node-with-changes: 295.44 ./node-original: 297.76 ................ -0.78%
http/simple.js type=buffer length=4 c=50: ./node-with-changes: 2513.6 ./node-original: 2295.8 ...................... 9.49%
http/simple.js type=buffer length=4 c=500: ./node-with-changes: 1952.6 ./node-original: 1919.5 ..................... 1.72%
http/simple.js type=buffer length=1024 c=50: ./node-with-changes: 2333 ./node-original: 2196.6 ..................... 6.21%
http/simple.js type=buffer length=1024 c=500: ./node-with-changes: 1990.3 ./node-original: 1939 .................... 2.64%
http/simple.js type=buffer length=102400 c=50: ./node-with-changes: 1291.6 ./node-original: 1264.4 ................. 2.16%
http/simple.js type=buffer length=102400 c=500: ./node-with-changes: 1237.2 ./node-original: 1215.8 ................ 1.76%

!state.bufferProcessing &&
state.buffer.length)
clearBuffer(this, state);
} else {
Copy link

Choose a reason for hiding this comment

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

This error message is kind of weird. Also, why nextTick? Seems like it should emit right now. Maybe something like 'Cannot uncork, not corked'?

@indutny
Copy link
Member Author

indutny commented Apr 26, 2013

@isaacs updated, following tips from our previous discussion. Please check and let me know if I forgot anything.

@isaacs
Copy link

isaacs commented Apr 27, 2013

Still a few nits to pick...

  1. writev() should get an array of {chunk,encoding} objects, rather than 2 parallel arrays. (Ie, Writable should just pass the buffer list, more or less as-is, I think.)
  2. long variadic functions are a no-no.

I'll add a patch tomorrow to help out with this.

@isaacs
Copy link

isaacs commented Apr 27, 2013

Top two patches here change the API to what we decided on the mailing list: https://github.com/isaacs/node/commits/writev

It'd still be good to get rid of the variadic functions and long functions. It's better to be less dry than to have variable function signatures or functions that are too long, or return early. This is hot code, need to kiss up to V8.

@indutny
Copy link
Member Author

indutny commented Apr 27, 2013

@isaacs thanks, I've merged your changes into my branch. However I took slightly different approach for net.js and made writev() accept pairs of data/encoding as an argument (for consistency reasons): handle.writev([ [data, enc], [data, enc], ... ]).

@isaacs isaacs merged commit f2d5cea into nodejs:master Apr 28, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants