dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
34.89k stars 9.85k forks source link

SignalR performance: Json is faster than MessagePack #31793

Open foxstream528 opened 3 years ago

foxstream528 commented 3 years ago

Describe the bug

I'm facing an issue with SignalR. Our benchmark shows that json protocol is faster than messagepack when transfering a lot of data. We need to investigate why messagepack isn't getting the same or better performance than json.

While signalR with json protocol bandwidth increases when using larger packets (following the performance of raw binary websocket implementation), signalR with messagepack seems to be capped. image

To Reproduce

Use the following code: https://github.com/Foxstream/SignalrMessagepackExample

Further technical details

It is a .NET 5.0 project with Microsoft.AspNetCore.SignalR.Client 5.0.5 and Microsoft.AspNetCore.SignalR.Protocols.MessagePack 5.0.5 packages. Running on Windows 10 20H2. It uses websocket transport protocol and HTTP.sys.

Using the .NET or the Javascript signalR client doesn't seem to matter because it will result in similar performance issues.

Results on localhost

With messagepack (2000 packets of 256KB)
179 packets/s | Bandwidth=45938 KB/s
It took 11.14s

With json (2000 packets of 256KB)
343 packets/s | Bandwidth=87885 KB/s
It took 5.82s
davidfowl commented 3 years ago

Any reason why the server is using HTTP.sys? Also, that's pretty impressive, good job JSON 😄

davidfowl commented 3 years ago

Also this pattern is broken https://github.com/Foxstream/SignalrMessagepackExample/blob/f5b24f6f88e3eab63ccf1ad6428f135363e9a69a/SignalrMessagepackExample/Startup.cs#L71-L81. You can't reuse the byte[] here, you have no idea when it's been use consumed so you need to allocate a new byte[] everytime to be safe.

Streaming large binary payloads over SignalR will also have a negative affect your GC behavior. What exactly are you streaming and can you shrink the package size so you don't end up with lots of temporary LOH allocations.

foxstream528 commented 3 years ago

Thank you for your quick feedback ^^'

That's a good question. If my memory serves me right, we use HTTP.sys to avoid having a limited number of simultaneous connections (but I am not sure of it, I will investigate).

To simplify the example project, I have directly use the byte[], but in our main project, we instanciate an object.

[MessagePackObject]
public class VideoDataResponse
{
      [Key("fragmentNumber")]
      public int FragmentNumber { get; set; }

      [Key("data")]
      public byte[] Data { get; set; }
}

We stream video data, so I do not think we can really shrink the package size.

edit: I have fixed the byte[] allocation in the example project. It doesn't seem to change anything about signalR performance. If you have a general idea of the problem, feel free to make suggestions, we can investigate on our side and possibly make a Pull Request if we find the issue.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

BrennanConroy commented 3 years ago

Thanks for the repro app. I first tried with Kestrel and didn't see the huge perf difference. But after switching to Http.Sys I can see the same perf hit.

BrennanConroy commented 3 years ago

image Well here is the problem, with MessagePack payloads are sent in 4k chunks, whereas with Json payloads are sent in 64k chunks. Still need to figure out why.

foxstream528 commented 3 years ago

We investigate on our side. We were not able to reproduce your analysis, but we found something else: there are "websocket continuation".

With HTTP.sys

With Kestrel

image

It seems websocket continuation is related to some sort of handshake because of unknown size packet. We think it may be related to the performance degradation. What do you think?

In any case, do you know if there are workarounds?

BrennanConroy commented 2 years ago

HttpSys wraps the output stream directly and just writes the data inline as it gets it. Another server like Kestrel, wraps the output stream in a much more complicated way and essentially buffers the writes so they go out in bigger chunks.

Backlogging as we wont be doing anything in 6.0 here.

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.