search

uber-archive / multitransport-jsonrpc

JSON-RPC Client (Node.js & Browser) and Server (Node.js) aim at "natural looking" server and client code.
116 stars 22 forks source link

Arrayify TCP #11

Closed dfellis closed 11 years ago

dfellis commented 11 years ago

So the array-based buffering concept didn't produce the desired/expected perf improvements for large messages.

@squamos can you take a look at the algorithm (and simplified Pascal string-style message format) and see if you spot anything funky with what I'm doing?

dfellis commented 11 years ago

Oh, this is what I'm getting in full-stack.js:

full-stack.js
✔ loopbackHttp
✔ failureTcp
TCP took 1018ms, 9823.182711198428 reqs/sec
HTTP took 12653ms, 790.3264048051846 reqs/sec
✔ perf_simple
TCP took 908ms, 11013.215859030837 reqs/sec
HTTP took 12539ms, 797.5117632985086 reqs/sec
✔ perf_100
TCP took 1054ms, 9487.666034155598 reqs/sec
HTTP took 13829ms, 723.1180851833104 reqs/sec
✔ perf_1000
TCP took 3909ms, 2558.1990278843696 reqs/sec
HTTP took 12916ms, 774.233508826262 reqs/sec
✔ perf_10000
TCP took 20535ms, 486.9734599464329 reqs/sec
HTTP took 13033ms, 767.2830507174097 reqs/sec
✔ perf_100000

This is the current implementation:

full-stack.js
✔ loopbackHttp
✔ failureTcp
TCP took 1029ms, 9718.172983479106 reqs/sec
HTTP took 12522ms, 798.5944737262419 reqs/sec
✔ perf_simple
TCP took 807ms, 12391.573729863692 reqs/sec
HTTP took 13204ms, 757.346258709482 reqs/sec
✔ perf_100
TCP took 899ms, 11123.470522803114 reqs/sec
HTTP took 12479ms, 801.3462617196891 reqs/sec
✔ perf_1000
TCP took 2751ms, 3635.0418029807342 reqs/sec
HTTP took 12278ms, 814.4648965629582 reqs/sec
✔ perf_10000
TCP took 38669ms, 258.6050841759549 reqs/sec
HTTP took 13575ms, 736.6482504604052 reqs/sec
✔ perf_100000

The bottom doesn't drop out quite as bad, but it's still not equal to HTTP like we'd expect.

Once I fix all of the tests to use the new message format, we can merge this in (it's still an ~88% improvement at the top-end with no measureable impact on the bottom-end) and investigate the rolling buffer concept.

dfellis commented 11 years ago

I had to cut out the corruption tests because the simplified messaging format has no built-in error checking and relies solely on TCP's error detection and correction.

dfellis commented 11 years ago

Haven't done Buffer.concat yet since it'll be a pretty major surgery on the algorithm, but here's a gist of the performance right now along with the v8 profile.

Specifically this line, Node is spending almost 50% of its time allocating memory for strings, and only about 0.1% of its time in the actual Javascript code. Is this a limitation of the Net library that we're running into?

dfellis commented 11 years ago

Some detective work; here are the results:

  1. Node's http lib appears to be entirely in Javascript built on top of the net module.
  2. We aren't "officially" doing anything wrong, it turns out net has undocumented APIs that http uses to get better performance.
  3. Confirmed that these interfaces have been stable for at least 0.8 -> 0.10, so I'm going to use it. It should mean that for small messages we do no buffer copying and for large messages we do far less copies, which should hopefully boost performance much higher.