-
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 4 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 |
|---|---|---|
|
|
@@ -8,8 +8,8 @@ namespace ModelContextProtocol.Protocol; | |
| /// <remarks> | ||
| /// <para> | ||
| /// <see cref="BlobResourceContents"/> is used when binary data needs to be exchanged through | ||
| /// the Model Context Protocol. The binary data is represented as a base64-encoded string | ||
| /// in the <see cref="Blob"/> property. | ||
| /// the Model Context Protocol. The binary data is represented as base64-encoded UTF-8 bytes | ||
| /// in the <see cref="Blob"/> property, providing a zero-copy representation of the wire payload. | ||
| /// </para> | ||
| /// <para> | ||
| /// This class inherits from <see cref="ResourceContents"/>, which also has a sibling implementation | ||
|
|
@@ -22,9 +22,47 @@ namespace ModelContextProtocol.Protocol; | |
| /// </remarks> | ||
| public sealed class BlobResourceContents : ResourceContents | ||
| { | ||
| private byte[]? _decodedData; | ||
| private ReadOnlyMemory<byte> _blob; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the base64-encoded string representing the binary data of the item. | ||
| /// Gets or sets the base64-encoded UTF-8 bytes representing the binary data of the item. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This is a zero-copy representation of the wire payload of this item. Setting this value will invalidate any cached value of <see cref="Data"/>. | ||
| /// </remarks> | ||
| [JsonPropertyName("blob")] | ||
| public required string Blob { get; set; } | ||
| public required ReadOnlyMemory<byte> Blob | ||
| { | ||
| get => _blob; | ||
| set | ||
| { | ||
| _blob = value; | ||
| _decodedData = null; // Invalidate cache | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the decoded data represented by <see cref="Blob"/>. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Accessing this member will decode the value in <see cref="Blob"/> and cache the result. | ||
| /// Subsequent accesses return the cached value unless <see cref="Blob"/> is modified. | ||
| /// </remarks> | ||
| [JsonIgnore] | ||
| public ReadOnlyMemory<byte> Data | ||
| { | ||
| get | ||
| { | ||
| if (_decodedData is null) | ||
| { | ||
| #if NET6_0_OR_GREATER | ||
|
||
| _decodedData = Convert.FromBase64String(System.Text.Encoding.UTF8.GetString(Blob.Span)); | ||
| #else | ||
| _decodedData = Convert.FromBase64String(System.Text.Encoding.UTF8.GetString(Blob.ToArray())); | ||
|
||
| #endif | ||
| } | ||
| return _decodedData; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -391,7 +391,7 @@ 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 = System.Text.Encoding.UTF8.GetBytes(dc.Base64Data.ToString()) }], | ||
| }, | ||
|
|
||
| string text => new() | ||
|
|
@@ -420,7 +420,7 @@ public override async ValueTask<ReadResourceResult> ReadAsync( | |
| { | ||
| Uri = request.Params!.Uri, | ||
| MimeType = dc.MediaType, | ||
| Blob = dc.Base64Data.ToString() | ||
| Blob = 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.