search

microsoft / vs-streamjsonrpc

The StreamJsonRpc library offers JSON-RPC 2.0 over any .NET Stream, WebSocket, or Pipe. With bonus support for request cancellation, client proxy generation, and more.
Other
740 stars 149 forks source link

Severe performance degradation with Server running on dotnet core 3.1 instead of dotnet core 2.2 #609

Closed dev-eureka closed 3 months ago

dev-eureka commented 3 years ago

Using the Server/Client sample provided and slightly modifying it to use MessagePack formatter instead of Json, I noticed a large performance degradation when running the server on dotnet core 3.1 vs 2.2.

The test is doing a transfer of a byte array of 512 KB (static in memory). The API is not using Steam, simply passing a byte[] as returned value. That same request is done 2,000 times in a loop and average timing is calculated:

I tried different versions of StreamJsonRPC from latest (2.6.121) to the first release (2.3.99) supporting dotnet core 3 and MessagePack. I am seeing the same behavior.

Extra tests done:

AArnott commented 3 years ago

I noticed a perf degradation lately as well. Let's use this issue to track it, although ultimately we may be forwarding to the .NET Core team. I have a benchmark that already called this out, so we have ETW traces which the .NET Core team will find useful in an analysis.

dev-eureka commented 3 years ago

@AArnott Glad to see that you observed it as well!

Are you able to get better performance with Stream as input/output instead of MessagePack? I tried it and the performance is actually worse.

AArnott commented 3 years ago

MessagePack vs. Stream isn't a choice. MessagePack vs. JSON is one choice, and Stream vs. IDuplexPipe is the other choice. So I'm not sure what you are hoping I'll measure, but I'll start with the existing benchmarks as I suspect it's related either way.

dev-eureka commented 3 years ago

Sorry I was not clear on this. An API method can receives either objects or streams. The objects will be serialized with Json/MessagePack and Stream will be sent more as pure binary without serialization. As it is mentioned in the doc, I would expect the streams to be much faster than going through Json/MessagePack serialization, but this is not what I observed.

My goal is to sent a byte[] of 512KB the fastest way. I tried to include it in an object and be serialized with Json/MessagePack and also tried through a stream directly. An object with MessagePack serialization is still the fastest for me. Stream was slower regardless of dotnet core version.

AArnott commented 3 years ago

I'm afraid it isn't repro'ing in the Benchmarks we have now, although I'm seeing netcoreapp2.1 mysteriously show a lot fewer allocations than net472 or netcoreapp3.1.

Can you try writing a benchmark in our existing benchmark project that shows off the problem?

dev-eureka commented 3 years ago

I am not really familiar with the way your benchmark are done. I am attaching my simple test application based on the provided sample. It is using StreamJsonRpc 2.5.46.

Server on dotnet core 2.2 (Client on dotnet core 3.1) generates this results:

Total Transfered: 1000 MB
Total Execution Time: 5251 ms
Execution Time per image: 2 ms
Bandwith: 190.43991620643686 MB/s

Server on dotnet core 3.1 (Client on dotnet core 3.1) generates this results:

Total Transfered: 1000 MB
Total Execution Time: 17279 ms
Execution Time per image: 8 ms
Bandwith: 57.87371954395509 MB/s

Source code of the test application: BenchmarkStreamJsonRPC.zip

Let me know if it helps you reproduce it.

AArnott commented 3 years ago

One possibility is that we have inappropriate #if NETCOREAPP filters on some of our optimized code that selects for netcoreapp2.1 but not 3.1. One such example that does presumably impact streamjsonrpc scenarios is: https://github.com/neuecc/MessagePack-CSharp/pull/1107 Brought up through Nerdbank.Streams via https://github.com/AArnott/Nerdbank.Streams/pull/284

AArnott commented 3 years ago

@dev-eureka Thank you for the repro. I haven't had time to test it yet, but would you mind trying your repro after adding this package reference to your project?

<PackageReference Include="Nerdbank.Streams" Version="2.6.84" />

You'll also need to add this to your nuget.config file, since the above version hasn't been pushed to nuget.org yet:

<add key="PublicCI" value="https://pkgs.dev.azure.com/andrewarnott/OSS/_packaging/PublicCI/nuget/v3/index.json" />
dev-eureka commented 3 years ago

Here are the results of the different tests:

I also did an extra MessagePack test (loop over 2000 images that serialize/deserialize the image of 512KB):

AArnott commented 3 years ago

Thanks. So, it looks like just sending byte[] (or equivalent) over messagepack is by far more efficient than using Stream. Interesting. However only Stream allows for streaming the data, allowing for arbitrarily large transfers. And I'd certainly like to see those transfers get faster. I'll continue the investigation using your repro then.

dev-eureka commented 3 years ago

@AArnott Were you able to replicate the issue on your side? Any findings?

AArnott commented 3 years ago

I'm afraid I haven't had any more time to look into this. If you do, please share anything more that you find. ETL traces would be superb, and if you find the problem is in the .NET Core runtime itself, I expect they would very much like to see a bug filed against their repo.