Zaid-Ajaj / Fable.Remoting

Type-safe communication layer (RPC-style) for F# featuring Fable and .NET Apps
https://zaid-ajaj.github.io/Fable.Remoting/
MIT License
273 stars 55 forks source link

Implement binary responses that bypass deserialization #117

Closed Zaid-Ajaj closed 5 years ago

Zaid-Ajaj commented 5 years ago

Addresses #116

Implements byte[] responses that are read directly as UInt8Array and thus bypassing the usual pipeline of converting first to base64 string just to read it back to an array of bytes.

The condition for binary responses is that the output type for a route must be Async<byte[]> and that the request includes a dummy header "x-remoting-proxy" so that old proxies and dotnet client stay compatible.

Added tests for binary routes to new proxy and old dotnet client to make sure these still work.

@0x53A The input parameters are still serialized to JSON even they are byte array. For only I want to verify the performance gains from bypassing byte array deserialization for large arrays

To benchmark the differences, use one route that returns Async<byte[]> and another that returns Async<Option<byte[]>>. The content is the same but the latter will go through deserialization while the former is converted directly

Zaid-Ajaj commented 5 years ago

@0x53A if I start rolling out new packages with the bypass, will you be able to test/benchmark them against your use-cases?

0x53A commented 5 years ago

Yeah. Not in my main project (Fable.Core 3 again), but should be easy to extract to a small benchmark programm.

Zaid-Ajaj commented 5 years ago

It's settled then, packages are published and will be available when nuget does it's magic. I look forward to hearing your findings :smile: if the performance gains are good and everyone is happy, I can then take a look at optimizing the binary inputs (i.e byte[]) to bypass input deserialization on the server (and serialization from the client)

0x53A commented 5 years ago

So I've finally had time and created a small benchmark (will upload to github in a few minutes).


At first I accidentally ran it with old server and new client.

It seems like this combination does not work correctly - both server functions (slow and fast) return the same byte array, but on the client I receive byte arrays with differing lengths:

grafik

grafik

grafik

So fast path with old server will return a blob that is too big. Reason is probably that it just returns the JSon un-deserialized.

Not sure if that is a scenario you want to support, if so, you'd need to add a header to the response, just like you did with the request.


0x53A commented 5 years ago

Did you also upload the server nuget? I can't find any pre-release

grafik

0x53A commented 5 years ago

Hm, according to the github PR view, 3.5.0 should contain the changes, but my above test (where I thought I had an old server) was with 3.5.0.

0x53A commented 5 years ago

repo is here: https://github.com/0x53A/fable-remoting-117

I'm tired and probably just made a silly mistake. Will look again at all this in the morning.

Zaid-Ajaj commented 5 years ago

Thanks a lot for the repro, I will check it right away.

So fast path with old server will return a blob that is too big. Reason is probably that it just returns the JSon un-deserialized. Not sure if that is a scenario you want to support, if so, you'd need to add a header to the response, just like you did with the request.

Old server, new client is not a scenario I accounted for. Only the other way around, New server is still compatible with old clients (both Fable and Dotnet). I will fix that in the Fable client

Hm, according to the github PR view, 3.5.0 should contain the changes, but my above test (where I thought I had an old server) was with 3.5.0.

I didn't push a pre-release because no breaking change was introduced, so 3.5.0 is the most recent version

0x53A commented 5 years ago

Giraffe checks x-binary-content grafik

Client sends x-remoting-proxy

grafik

0x53A commented 5 years ago

You are fast, now NuGet is the bottleneck ;D

Zaid-Ajaj commented 5 years ago

My bad :( the fix is already published. Again, my integration tests are running with the Suave server where I manually checked the responses and it all worked out but it seems one can never have enough tests

0x53A commented 5 years ago

[...] but it seems one can never have enough tests

Yeah


Okay real test:

fast path is 25% smaller and also 20%-30% faster (3 seconds instead of 4 in firefox)

grafik

grafik

0x53A commented 5 years ago

Chrome is a bit faster in general, but here the fast path is also notacibly faster:

grafik

grafik

grafik

As you can see the slow path has a big CPU usage block after the network request, where the fast path doesn't

0x53A commented 5 years ago

This looks really good, thank you! 🎆 🐧

0x53A commented 5 years ago

Just checked, it's not as much, but skipping serialization is also slightly (~100ms) faster on the server:

grafik

Zaid-Ajaj commented 5 years ago

I just reproduced these results and . . . wow! Not only the CPU time was reduced, the network latency too, man I am happy now :smile: I guess I will start working on optimizing binary inputs too very soon

On a side note, I am very intrigued to see someone pushing the library to it's limits. If I may ask, what kind of application are you building, is it a game, maybe even a game engine?

0x53A commented 5 years ago

Nope not a game. Not public yet, but I'd be happy to send you a DM on twitter or somewhere else.

(But it IS 3D, and I use babylonjs )

Zaid-Ajaj commented 5 years ago

Sounds like an a cool project! Also DMs are always open ;)

0x53A commented 5 years ago

It is cool, Fable is really fun! (They should have kept the name FunScript ;) ) - I did send you one with a link, and we'll present it publically in ~2 weeks. Good night :D 💤