deno_tcp latency is worse than node_tcp

[ry]

Thanks to @bartlomieju's work in #2689 we're able to clearly see now that deno_tcp has worse 99% tail latency than node_tcp.

Previously I believed that our max latency was on par with Node's. Back in April, @piscisaureus did work to improve our latency numbers, but apparently it was not enough.

This is a very good place to investigate performance problems.

Note that deno_core_single does not exhibit the latency problems! deno_core_multi is worse, but not as bad as deno_tcp....

[bartlomieju]

Ref #2758

[bartlomieju]

Interestingly deno_http has 4x lower latency than deno_tcp!

[lucacasonato]

Can this be closed? deno_tcp averages at about the same latency, or slightly better than node_tcp now. https://deno.land/benchmarks?-50#http-latency

[ry]

Yes - this is fixed.

[hayd]

there appears to be a relatively recent (?) regression:

https://deno.land/benchmarks?-100#http-latency

[AaronO]

This divergence seems to have started with fec1b2a (first commit changing the OP layer)

So this is on my radar, but it's curious that there's a big regression in latency. I'll try to investigate this and correct it for 1.9 (and ideally exceed the original latency)

[AaronO]

@hayd I believe I've identified why performance & latency regressed with fec1b2a.

TLDR: serde_json::Values are very inefficient with serde_v8 (take all slow paths)

Ultimately it's a combination of a few factors that should not be present in 1.9:

• rumtime/tcp ops still use serde_json-isms and take/return serde_json::Values
• These are loosely-typed maps, which take serde_v8's slowpaths
  • Can't use v8_struct_key so each field allocates a new v8 string on heap (will cause GC pressure)
  • This happens both for input args and output returns
  • Each key has to go through v8's string write utf8 which converts the strings
• Serializing/deserializing Maps hasn't been optimized at all in serde_v8 and in fact has no benches (yet)
  • In fact there's a known inefficiency in deserialize_map (which doubles v8::Object::Get calls), added to drop keys that exists but whose values are undefined which is what serde_json did and is behaviour that we relied on, at least in tests
• Plus the overhead of allocating the unnecessary serde_json::Value

So removing these serde_json-isms like I've done in #10009 should fix the regression and likely deliver better performance.

[AaronO]

After further digging, the P99 latency increase appears to be caused by the promiseTable change.

#9843 changed the promiseTable from a JS Object to a Map, which causes higher GC-pressure and thus higher tail latencies.

Checking out the promise ring (#9979), hugely closed the P99 delta between node and deno on the tcp bench

[AaronO]

@ry @hayd I would consider this closed with the op improvements that shipped in 1.9.0, deno_tcp has lower max latency than node_tcp (I still think we should use P99 or something instead of max, max isn't a meaningful statistical measurement and can be caused by a single outlier) current chart: