Improper use of RecyclableMemoryStream

[pahlot]

While the use of RecyclableMemoryStream is to be commended, the implementation negates any advantage of using it due to the use of ToArray().

Use GetBuffer or GetMemory instead.

Also, converting json to strings seems rather unnecessary as it just causes more allocations.

Directly from the RecyclableMemoryStream documentation;

ToArray - It looks similar to GetBuffer on the surface, but is actually significantly different. In ToArray the data is always copied into a new array that is exactly the right length for the full contents of the stream. This new buffer is never pooled. Users of this library should consider any call to ToArray to be a bug, as it wipes out many of the benefits of RecyclableMemoryStream completely. However, the method is included for completeness, especially if you are calling other APIs that only take a byte array with no length parameter. An event is logged on all ToArray calls.

[Odonno]

While the use of RecyclableMemoryStream is to be commended, the implementation negates any advantage of using it due to the use of ToArray().

Use GetBuffer or GetMemory instead.

Yes indeed. The fact is that there are 3 use cases for this MemoryStream:

1. Text Message (string type)
2. Binary Message (byte[] type)
3. BInary Message, providing a MemoryStream

The last case 3. is the most efficient as you can use the GetBuffer method in your code. Note that you will have to dispose the stream manually and pass IsStreamDisposedAutomatically = false;

For 1., I made a PR to use GetBuffer as you suggested. We can indeed consume the stream instead of having a temporary byte[] created in memory.

For 2., we have to pass a byte[] because it is the type we pass by. And because the stream will be disposed automatically, we can no longer provide an access to the stream. For performance reason, you should always choose the 3. strategy.

Also, converting json to strings seems rather unnecessary as it just causes more allocations.

What are you referring to?

[pahlot]

Also, converting json to strings seems rather unnecessary as it just causes more allocations.

What are you referring to?

I'm referring to both Newtonsoft and System.Text.Json being able to accept streams, or in STJ's instance ReadonlySpans as well. If performance is a concern in this package (it may not be, it may be secondary to ease of use by novice developers), then persisting to string makes little sense from a performance perspective, not to mention payloads larger than 85kb ending up on the LOH.

[Odonno]

You can set IsTextMessageConversionEnabled = false and read the Stream like I said with option 3 for maximum performance.

[pahlot]

I must have missed that part of the API. Thanks for clarifying!