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

Pluggable message encoding? #50

Open ssimeonov opened 10 years ago

ssimeonov commented 10 years ago

It's great that the module offers pluggable transports. Any thoughts on breaking serialization/deserialization out of the transport so that, for example, collaborating clients & servers that support more efficient encodings such as MsgPack could switch to that?

I guess that may require changing the module name to multitransport-rpc :).

dfellis commented 10 years ago

mulitransport-multirpc!

But more seriously, we did investigate MsgPack and found it was more CPU intensive than JSON-RPC in Node. Granted this was in node 0.8 days and you'll still have bandwidth gains from non-string-heavy RPC loads, but inside of a datacenter the bandwidth wasn't as much of an issue as the CPU burn to communicate between node processes.

You're right that it would need to be renamed for multiple serialization formats, so feel free to fork it and rename it. :)

Raynos commented 10 years ago

or you can just pass in a { stringify: Function, parse: Function } option and default to JSON, should be a simple PR to open :)

ssimeonov commented 10 years ago

@Raynos I was thinking along the same lines, perhaps even passing a marshal option whose default value is the JSON object but anything else which provides stringify and parse could be used. It doesn't have to be much more complicated than that.

ssimeonov commented 10 years ago

@dfellis that is a very interesting finding. I'm curious: how did you compare the CPU intensiveness of MsgPack vs. JSON encoding? Were there throughput differences?

dfellis commented 10 years ago

I don't remember the details too well. @squamos was the one who did most of the testing with MsgPack, but the increased CPU load would automatically imply a lower throughput cap.

As I recall, MsgPack works best on arrays of floats, but strings obviously do not compress in size (so objects could never compact their keys) and very small integers (-99 - 999) would take less space in JSON than the MsgPack equivalent (1-3 bytes versus 4). The minimal packing gains coupled with the higher barrier to entry if we ever wanted to expose an endpoint publicly meant we just dropped the concept and haven't looked back.

dfellis commented 10 years ago

I have no problem with making it possible to provide a "substitute JSON". It will be a bit more work because now the code will have to check if the output type is a String or a Buffer and act accordingly when sending it down the wire (setting proper encoding headers for one or the other in HTTP, making sure getting the length and packing in the bytes is done correctly for Buffers in TCP, doing I don't know what with Node's built-in IPC mechanism [maybe nothing], and probably doing nothing for the Loopback transport), so I don't think it will be quite as simple a PR as @Raynos believes. :)

ssimeonov commented 10 years ago

@dfellis as I was looking at MsgPack over TCP I did not consider the complexities over HTTP. You are right, this probably does not belong here.

As for integer representations in MsgPack, they do have a number of special types to represent small numbers so it's definitely far smaller than JSON. Overall, though, you are absolutely right: key-value data types will not compact as well.