-
Notifications
You must be signed in to change notification settings - Fork 587
Use ReadOnlyMemory<byte> for binary data to eliminate UTF-16 transcoding #1070
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?
Changes from 5 commits
7dca602
4287694
8e6fcf0
e405dfc
ebe3eef
1d76c11
39213fb
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 |
|---|---|---|
|
|
@@ -264,9 +264,9 @@ public static IList<PromptMessage> ToPromptMessages(this ChatMessage chatMessage | |
| { | ||
| TextContentBlock textContent => new TextContent(textContent.Text), | ||
|
|
||
| ImageContentBlock imageContent => new DataContent(Convert.FromBase64String(imageContent.Data), imageContent.MimeType), | ||
| ImageContentBlock imageContent => new DataContent(imageContent.DecodedData, imageContent.MimeType), | ||
|
|
||
| AudioContentBlock audioContent => new DataContent(Convert.FromBase64String(audioContent.Data), audioContent.MimeType), | ||
| AudioContentBlock audioContent => new DataContent(audioContent.DecodedData, audioContent.MimeType), | ||
|
|
||
| EmbeddedResourceBlock resourceContent => resourceContent.Resource.ToAIContent(), | ||
|
|
||
|
|
@@ -307,7 +307,7 @@ public static AIContent ToAIContent(this ResourceContents content) | |
|
|
||
| AIContent ac = content switch | ||
| { | ||
| BlobResourceContents blobResource => new DataContent(Convert.FromBase64String(blobResource.Blob), blobResource.MimeType ?? "application/octet-stream"), | ||
| BlobResourceContents blobResource => new DataContent(blobResource.Data, blobResource.MimeType ?? "application/octet-stream"), | ||
| TextResourceContents textResource => new TextContent(textResource.Text), | ||
| _ => throw new NotSupportedException($"Resource type '{content.GetType().Name}' is not supported.") | ||
| }; | ||
|
|
@@ -380,21 +380,21 @@ public static ContentBlock ToContentBlock(this AIContent content) | |
|
|
||
| DataContent dataContent when dataContent.HasTopLevelMediaType("image") => new ImageContentBlock | ||
| { | ||
| Data = dataContent.Base64Data.ToString(), | ||
| Data = System.Text.Encoding.UTF8.GetBytes(dataContent.Base64Data.ToString()), | ||
|
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. DataContent already has |
||
| MimeType = dataContent.MediaType, | ||
| }, | ||
|
|
||
| DataContent dataContent when dataContent.HasTopLevelMediaType("audio") => new AudioContentBlock | ||
| { | ||
| Data = dataContent.Base64Data.ToString(), | ||
| Data = System.Text.Encoding.UTF8.GetBytes(dataContent.Base64Data.ToString()), | ||
|
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. Same, can we use |
||
| MimeType = dataContent.MediaType, | ||
| }, | ||
|
|
||
| DataContent dataContent => new EmbeddedResourceBlock | ||
| { | ||
| Resource = new BlobResourceContents | ||
| { | ||
| Blob = dataContent.Base64Data.ToString(), | ||
| Blob = System.Text.Encoding.UTF8.GetBytes(dataContent.Base64Data.ToString()), | ||
|
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 think this should just be |
||
| MimeType = dataContent.MediaType, | ||
| Uri = string.Empty, | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| using System.Globalization; | ||
| using System.Reflection; | ||
| using System.Runtime.CompilerServices; | ||
| using System.Runtime.InteropServices; | ||
| using System.Text; | ||
| using System.Text.Json; | ||
| using System.Text.Json.Nodes; | ||
|
|
@@ -391,7 +392,14 @@ public override async ValueTask<ReadResourceResult> ReadAsync( | |
|
|
||
| DataContent dc => new() | ||
| { | ||
| Contents = [new BlobResourceContents { Uri = request.Params!.Uri, MimeType = dc.MediaType, Blob = dc.Base64Data.ToString() }], | ||
| Contents = [new BlobResourceContents | ||
| { | ||
| Uri = request.Params!.Uri, | ||
| MimeType = dc.MediaType, | ||
| Blob = MemoryMarshal.TryGetArray(dc.Base64Data, out ArraySegment<char> segment) && segment.Offset == 0 && segment.Count == segment.Array!.Length | ||
|
||
| ? System.Text.Encoding.UTF8.GetBytes(segment.Array) | ||
| : System.Text.Encoding.UTF8.GetBytes(dc.Base64Data.ToString()) | ||
| }], | ||
| }, | ||
|
|
||
| string text => new() | ||
|
|
@@ -420,7 +428,9 @@ public override async ValueTask<ReadResourceResult> ReadAsync( | |
| { | ||
| Uri = request.Params!.Uri, | ||
| MimeType = dc.MediaType, | ||
| Blob = dc.Base64Data.ToString() | ||
| Blob = MemoryMarshal.TryGetArray(dc.Base64Data, out ArraySegment<char> segment) && segment.Offset == 0 && segment.Count == segment.Array!.Length | ||
| ? System.Text.Encoding.UTF8.GetBytes(segment.Array) | ||
| : System.Text.Encoding.UTF8.GetBytes(dc.Base64Data.ToString()) | ||
| }, | ||
|
|
||
| _ => throw new InvalidOperationException($"Unsupported AIContent type '{ac.GetType()}' returned from resource function."), | ||
|
|
||
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: consider changing this to a span literal to avoid the need to parse a static resource. Both DataContent and ImageContentBlock can construct from the raw bytes.