[IMPROVEMENT PERFORMANCE] System.IO.MemoryStream replaced with Microsoft.IO.RecyclableMemoryStream#1197
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR replaces ad-hoc MemoryStream allocations with Microsoft.IO.RecyclableMemoryStream across the library and tests, introducing a shared MemoryStreamManager and adding the required package reference.
Changes:
- Add
Microsoft.IO.RecyclableMemoryStreamas a dependency and centralize its version. - Update production code paths that buffer JSON/response content to use
RecyclableMemoryStream. - Update tests/examples to use
RecyclableMemoryStreamManager.GetStream()instead ofnew MemoryStream().
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/UserAgentTests.cs | Switches empty request content stream to pooled recyclable stream |
| tests/Realtime/RealtimeTests.cs | Uses pooled stream for concurrent audio-send test |
| tests/Images/ImagesMockTests.cs | Replaces multiple MemoryStream allocations with recyclable streams |
| tests/Files/FilesMockTests.cs | Uses recyclable streams for request-body capture and cancellation tests |
| tests/Chat/ChatTests.cs | Uses recyclable streams for audio output buffering in streaming tests |
| tests/Batch/BatchTests.cs | Uses recyclable streams for batch input file serialization |
| tests/Audio/TranslationMockTests.cs | Uses recyclable stream for cancellation test input |
| tests/Audio/TranscriptionMockTests.cs | Uses recyclable stream for cancellation test input |
| examples/Chat/Example08_ChatSerialization.cs | Switches serialization buffer to recyclable stream and changes Utf8JsonWriter construction |
| OpenAI/src/Utility/RecyclableMemoryStreamManager.cs | Introduces shared recyclable stream manager wrapper |
| OpenAI/src/OpenAI.csproj | Adds package reference for recyclable streams |
| OpenAI/src/Generated/Internal/Utf8JsonBinaryContent.cs | Uses recyclable stream for JSON binary content buffering |
| OpenAI/src/Custom/Realtime/Internal/WebsocketPipelineResponse.cs | Uses recyclable stream for websocket response content buffering |
| OpenAI/src/Custom/Moderations/ModerationClient.cs | Uses recyclable stream for moderation input JSON serialization |
| OpenAI/src/Custom/FineTuning/Internal/Pagination/FineTuningEventsPageToken.cs | Uses recyclable stream for page token serialization |
| OpenAI/src/Custom/FineTuning/Internal/Pagination/FineTuningEventCollectionPageToken.cs | Uses recyclable stream for page token serialization |
| OpenAI/src/Custom/FineTuning/Internal/Pagination/FineTuningCheckpointCollectionPageToken.cs | Uses recyclable stream for page token serialization |
| OpenAI/src/Custom/FineTuning/Internal/FineTuningJobToken.cs | Uses recyclable stream for token serialization |
| OpenAI/src/Custom/Embeddings/EmbeddingClient.cs | Uses recyclable stream for embedding request JSON serialization |
| OpenAI/src/Custom/Batch/Internal/CreateBatchOperationToken.cs | Uses recyclable stream for token serialization |
| OpenAI/src/Custom/Assistants/MessageCreationAttachment.cs | Uses recyclable stream when serializing tool definitions |
| Directory.Packages.props | Adds centrally managed version for recyclable stream package |
| internal partial class Utf8JsonBinaryContent : BinaryContent | ||
| { | ||
| private readonly MemoryStream _stream; | ||
| private readonly Microsoft.IO.RecyclableMemoryStream _stream; |
There was a problem hiding this comment.
well
public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
| _stream = MemoryStreamManager.Manager.GetStream(); | ||
| _content = Create(_stream); | ||
| JsonWriter = new Utf8JsonWriter(_stream); | ||
| JsonWriter = new Utf8JsonWriter(_stream as IBufferWriter<byte>); |
There was a problem hiding this comment.
well
public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
| using Microsoft.IO.RecyclableMemoryStream stream = Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as IBufferWriter<byte>); |
There was a problem hiding this comment.
well
public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as IBufferWriter<byte>); |
There was a problem hiding this comment.
well
public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
| internal class MemoryStreamManager | ||
| { | ||
| public static readonly RecyclableMemoryStreamManager Manager = new (); | ||
| } No newline at end of file |
There was a problem hiding this comment.
well
public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
| using Microsoft.IO.RecyclableMemoryStream ms = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using (var tempWriter = new Utf8JsonWriter(ms as System.IO.Stream)) |
There was a problem hiding this comment.
well
public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
There was a problem hiding this comment.
Since we're not interested in the potential null outcome and an NRE would be confusing, let's please use a direct cast.
|
I am hitting API_KEY limits with tests. But some results |
| private readonly Microsoft.IO.RecyclableMemoryStream _stream; | ||
| private readonly BinaryContent _content; | ||
|
|
||
| public Utf8JsonBinaryContent() | ||
| { | ||
| _stream = new MemoryStream(); | ||
| _stream = MemoryStreamManager.Manager.GetStream(); | ||
| _content = Create(_stream); | ||
| JsonWriter = new Utf8JsonWriter(_stream); | ||
| JsonWriter = new Utf8JsonWriter(_stream as System.IO.Stream); | ||
| } |
| public override Stream ContentStream | ||
| { | ||
| get => _contentStream ??= new MemoryStream(); | ||
| get => _contentStream ??= MemoryStreamManager.Manager.GetStream(); | ||
| set => throw new NotImplementedException(); | ||
| } | ||
| private MemoryStream _contentStream; | ||
| private Microsoft.IO.RecyclableMemoryStream _contentStream; | ||
|
|
||
| public override BinaryData Content => _content ??= new(_contentStream.ToArray()); | ||
| private BinaryData _content; |
There was a problem hiding this comment.
I don't believe that Copilot is incorrect here, unfortunately. It looks like the WebsocketPipelineResponse is consumed only by AsyncWebsocketMessageEnumerator which does not currently expose a way to dispose, as it wraps the response in ClientResult<T> when handing back to callers via the enumerator.
ClientResult is not disposable and requires that callers hold responsibility for disposal of messages. While there is a disposal path in the pipeline for HTTP responses, this RealTime implementation appears to lack them.
| internal class MemoryStreamManager | ||
| { | ||
| public static readonly RecyclableMemoryStreamManager Manager = new (); | ||
| } No newline at end of file |
| using Microsoft.IO.RecyclableMemoryStream stream = Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as System.IO.Stream); |
| [Test] | ||
| public void UserAgentWithApplicationIdWorks() => UserAgentStringWorks(useApplicationId: true); | ||
|
|
||
| private static readonly Microsoft.IO.RecyclableMemoryStreamManager Manager = new (); |
jsquire
left a comment
There was a problem hiding this comment.
Hi @moljac. Thank you for your contribution and interest in improving the OpenAI developer experience.
While we're open to the proposed approach, we are also very intentional about introducing additional dependencies. Though this is a Microsoft-owned library, it is also one that has not received a release in 2 years, and which has very low activity from maintainers. It also introduces a non-trivial amount of complexity and risk of leaks unless we're ensuring that disposal is honored.
I'd like to understand the impact of this change in real-world scenarios to understand both the performance and memory characteristics. If we see an impact significant enough to warrant the additional dependency and complexity, we'll evaluate next steps for moving forward.
Would you be open to providing BenchmarkDotNet results for the some of our sample scenarios in each area that you're updating?
| } | ||
| #endregion | ||
|
|
||
| private static readonly Microsoft.IO.RecyclableMemoryStreamManager Manager = new (); |
There was a problem hiding this comment.
We'll definitely want to preserve use of the standard MemoryStream in examples to reduce complexity.
| { | ||
| using var ms = new System.IO.MemoryStream(); | ||
| using (var tempWriter = new Utf8JsonWriter(ms)) | ||
| using Microsoft.IO.RecyclableMemoryStream ms = OpenAI.MemoryStreamManager.Manager.GetStream(); |
There was a problem hiding this comment.
| using Microsoft.IO.RecyclableMemoryStream ms = OpenAI.MemoryStreamManager.Manager.GetStream(); | |
| using var ms = OpenAI.MemoryStreamManager.Manager.GetStream(); |
nit: Please preserve the existing form.
| using Microsoft.IO.RecyclableMemoryStream ms = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using (var tempWriter = new Utf8JsonWriter(ms as System.IO.Stream)) |
There was a problem hiding this comment.
Since we're not interested in the potential null outcome and an NRE would be confusing, let's please use a direct cast.
| { | ||
| using MemoryStream stream = new(); | ||
| using Utf8JsonWriter writer = new(stream); | ||
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); |
There was a problem hiding this comment.
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); | |
| using var stream = OpenAI.MemoryStreamManager.Manager.GetStream(); |
| using MemoryStream stream = new(); | ||
| using Utf8JsonWriter writer = new(stream); | ||
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as System.IO.Stream); |
There was a problem hiding this comment.
| using Utf8JsonWriter writer = new(stream as System.IO.Stream); | |
| using Utf8JsonWriter writer = new((MemoryStream)stream); |
| public override Stream ContentStream | ||
| { | ||
| get => _contentStream ??= new MemoryStream(); | ||
| get => _contentStream ??= MemoryStreamManager.Manager.GetStream(); | ||
| set => throw new NotImplementedException(); | ||
| } | ||
| private MemoryStream _contentStream; | ||
| private Microsoft.IO.RecyclableMemoryStream _contentStream; | ||
|
|
||
| public override BinaryData Content => _content ??= new(_contentStream.ToArray()); | ||
| private BinaryData _content; |
There was a problem hiding this comment.
I don't believe that Copilot is incorrect here, unfortunately. It looks like the WebsocketPipelineResponse is consumed only by AsyncWebsocketMessageEnumerator which does not currently expose a way to dispose, as it wraps the response in ClientResult<T> when handing back to callers via the enumerator.
ClientResult is not disposable and requires that callers hold responsibility for disposal of messages. While there is a disposal path in the pipeline for HTTP responses, this RealTime implementation appears to lack them.
| internal partial class Utf8JsonBinaryContent : BinaryContent | ||
| { | ||
| private readonly MemoryStream _stream; | ||
| private readonly Microsoft.IO.RecyclableMemoryStream _stream; |
There was a problem hiding this comment.
This is generated code that cannot be modified by hand. As the generator is an externally owned construct, this is not a path that you'll be able to update.
|
|
||
| namespace OpenAI; | ||
|
|
||
| internal class MemoryStreamManager |
There was a problem hiding this comment.
| internal class MemoryStreamManager | |
| internal static class MemoryStreamManager |
|
Sorry for delay. I was dragged away with my daily tasks. I will add comments. |
|
Hi @jsquire
Sure.
I do understand your concern. I noticed that some of state of the art tools made by Microsoft use this library. To name some:
more info: https://www.nuget.org/packages/Microsoft.IO.RecyclableMemoryStream#usedby-body-tab
M.IO.RMS reduces GC pressure by less Heap allocations. I was able to make few MAUI apps more performant and I have set of serialization/deserialization wrappers for several formats JSON, XML, CSV together with benchmarks and this library does help in quite a lot cases.
Sure. I will take a look. Until then I will make PR WIP. |
System.IO.MemoryStreamwas replaced withMicrosoft.IO.RecyclableMemoryStreamChanges:
Microsoft.IO.RecyclableMemoryStreamSystem.IO.MemoryStreaminstance occurences replaced withMicrosoft.IO.RecyclableMemoryStreamMicrosoft.IO.RecyclableMemoryStreamreplaces standardMemoryStreaminstances with pooled byte buffers, eliminating Large Object Heap (LOH) allocations and significantly reducing Garbage Collection (GC) pauses in high-throughput applicationshttps://github.com/microsoft/Microsoft.IO.RecyclableMemoryStream
[FEATURE] Utilize Microsoft.IO.RecyclableMemoryStream in flavor of creating new MemoryStream instances when serializing ZiggyCreatures/FusionCache#241
Utilities like Kiota do use
Microsoft.IO.RecyclableMemoryStreamas dependencyhttps://www.nuget.org/packages/Microsoft.Kiota.Serialization.Form/
https://www.nuget.org/packages/Microsoft.Kiota.Serialization.Json/
https://www.nuget.org/packages/Microsoft.Kiota.Serialization.Multipart/
https://www.nuget.org/packages/Microsoft.Kiota.Serialization.Text/