-
Notifications
You must be signed in to change notification settings - Fork 409
[IMPROVEMENT PERFORMANCE] System.IO.MemoryStream replaced with Microsoft.IO.RecyclableMemoryStream #1197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[IMPROVEMENT PERFORMANCE] System.IO.MemoryStream replaced with Microsoft.IO.RecyclableMemoryStream #1197
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,13 +2,15 @@ | |
| using System.ClientModel.Primitives; | ||
| using System.Collections.Generic; | ||
| using System.Text.Json; | ||
| using Microsoft.IO; | ||
|
|
||
| namespace OpenAI.Assistants; | ||
|
|
||
| [CodeGenType("CreateMessageRequestAttachment")] | ||
| [CodeGenSerialization(nameof(Tools), "tools", SerializationValueHook = nameof(SerializeTools), DeserializationValueHook = nameof(DeserializeTools))] | ||
| public partial class MessageCreationAttachment | ||
| { | ||
|
|
||
| /// <summary> | ||
| /// The tools to which the attachment applies to. | ||
| /// </summary> | ||
|
|
@@ -35,8 +37,8 @@ private void SerializeTools(Utf8JsonWriter writer, ModelReaderWriterOptions opti | |
| writer.WriteStartArray(); | ||
| foreach (ToolDefinition tool in Tools) | ||
| { | ||
| using var ms = new System.IO.MemoryStream(); | ||
| using (var tempWriter = new Utf8JsonWriter(ms)) | ||
| using Microsoft.IO.RecyclableMemoryStream ms = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using (var tempWriter = new Utf8JsonWriter(ms as System.IO.Stream)) | ||
|
Comment on lines
+40
to
+41
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're not interested in the potential
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. I will fix that. |
||
| { | ||
| tempWriter.WriteObjectValue(tool, options); | ||
| tempWriter.Flush(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||
| using System; | ||||||
| using System.Buffers; | ||||||
| using System.ClientModel; | ||||||
| using System.Diagnostics; | ||||||
| using System.IO; | ||||||
| using System.Text.Json; | ||||||
|
|
||||||
| #nullable enable | ||||||
|
|
@@ -19,8 +19,8 @@ public CreateBatchOperationToken(string batchId) | |||||
|
|
||||||
| public override BinaryData ToBytes() | ||||||
| { | ||||||
| using MemoryStream stream = new(); | ||||||
| using Utf8JsonWriter writer = new(stream); | ||||||
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| using Utf8JsonWriter writer = new(stream as IBufferWriter<byte>); | ||||||
| writer.WriteStartObject(); | ||||||
|
|
||||||
| writer.WriteString("batchId", BatchId); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| using Microsoft.TypeSpec.Generator.Customizations; | ||
| using System; | ||
| using System.Buffers; | ||
| using System.ClientModel; | ||
| using System.ClientModel.Primitives; | ||
| using System.Collections.Generic; | ||
|
|
@@ -238,8 +239,8 @@ public virtual ClientResult<ModerationResult> ClassifyInputs(IEnumerable<Moderat | |
|
|
||
| private void CreateModerationOptions(string input, ref ModerationOptions options) | ||
| { | ||
| using MemoryStream stream = new(); | ||
| using Utf8JsonWriter writer = new(stream); | ||
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as IBufferWriter<byte>); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit) |
||
|
|
||
| writer.WriteStringValue(input); | ||
| writer.Flush(); | ||
|
|
@@ -250,8 +251,8 @@ private void CreateModerationOptions(string input, ref ModerationOptions options | |
|
|
||
| private void CreateModerationOptions(IEnumerable<string> inputs, ref ModerationOptions options) | ||
| { | ||
| using MemoryStream stream = new(); | ||
| using Utf8JsonWriter writer = new(stream); | ||
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as IBufferWriter<byte>); | ||
|
|
||
| writer.WriteStartArray(); | ||
|
|
||
|
|
@@ -269,8 +270,8 @@ private void CreateModerationOptions(IEnumerable<string> inputs, ref ModerationO | |
|
|
||
| private void CreateModerationOptions(IEnumerable<ModerationInputPart> inputParts, ref ModerationOptions options) | ||
| { | ||
| using MemoryStream stream = new(); | ||
| using Utf8JsonWriter writer = new(stream); | ||
| using Microsoft.IO.RecyclableMemoryStream stream = OpenAI.MemoryStreamManager.Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as IBufferWriter<byte>); | ||
|
|
||
| writer.WriteStartArray(); | ||
| foreach (ModerationInputPart part in inputParts) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| using System.Net.WebSockets; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.IO; | ||
|
|
||
| namespace OpenAI.Realtime; | ||
|
|
||
|
|
@@ -25,10 +26,10 @@ internal class WebsocketPipelineResponse : PipelineResponse | |
|
|
||
| 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; | ||
|
Comment on lines
27
to
35
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe that Copilot is incorrect here, unfortunately. It looks like the
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| #nullable disable | ||
|
|
||
| using System.Buffers; | ||
| using System.ClientModel; | ||
| using System.IO; | ||
| using System.Text.Json; | ||
|
|
@@ -12,14 +13,14 @@ namespace OpenAI | |
| { | ||
| internal partial class Utf8JsonBinaryContent : BinaryContent | ||
| { | ||
| private readonly MemoryStream _stream; | ||
| private readonly Microsoft.IO.RecyclableMemoryStream _stream; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| 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 IBufferWriter<byte>); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit) |
||
| } | ||
|
Comment on lines
+16
to
24
|
||
|
|
||
| public Utf8JsonWriter JsonWriter { get; } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,8 @@ | ||||||
| using Microsoft.IO; | ||||||
|
|
||||||
| namespace OpenAI; | ||||||
|
|
||||||
| internal class MemoryStreamManager | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| { | ||||||
| public static readonly RecyclableMemoryStreamManager Manager = new (); | ||||||
| } | ||||||
|
Comment on lines
+5
to
+8
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit)
Comment on lines
+5
to
+8
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| using NUnit.Framework; | ||
| using OpenAI.Chat; | ||
| using System; | ||
| using System.Buffers; | ||
| using System.ClientModel.Primitives; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
|
|
@@ -23,11 +24,13 @@ public static IEnumerable<ChatMessage> DeserializeMessages(BinaryData data) | |
| } | ||
| #endregion | ||
|
|
||
| private static readonly Microsoft.IO.RecyclableMemoryStreamManager Manager = new (); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll definitely want to preserve use of the standard |
||
|
|
||
| #region | ||
| public static BinaryData SerializeMessages(IEnumerable<ChatMessage> messages) | ||
| { | ||
| using MemoryStream stream = new(); | ||
| using Utf8JsonWriter writer = new(stream); | ||
| using Microsoft.IO.RecyclableMemoryStream stream = Manager.GetStream(); | ||
| using Utf8JsonWriter writer = new(stream as IBufferWriter<byte>); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well public sealed class RecyclableMemoryStream : MemoryStream, IBufferWriter<byte>Tests were passing (until I hit limit) |
||
|
|
||
| writer.WriteStartArray(); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Please preserve the existing form.