Skip to content
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

Add Encode(ReadOnlySpan<char>) method to TextEncoder for performance #30274

Closed
steveharter opened this issue Jul 16, 2019 · 0 comments · Fixed by dotnet/corefx#39900
Closed
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Encodings.Web
Milestone

Comments

@steveharter
Copy link
Member

With the recent custom encoder work, it was found that the existing System.Text.Encodings.Web.TextEncoder class does not have an overload of an "Encode" method taking a ReadOnlySpan<char>. The causes a perf hit because the callers (System.Text.Json's writer and serializer) must convert utf16->utf8 (in order to call EncodeUtf8)->utf16.

Proposed API:

namespace System.Text.Encodings.Web
{
    public class TextEncoder // existing class
    {
        // Existing method as a reference:
        public virtual OperationStatus EncodeUtf8(ReadOnlySpan<byte> utf8Source, Span<byte> utf8Destination, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);

        // Proposed method:
        public virtual OperationStatus Encode(ReadOnlySpan<char> source, Span<char> destination, out int charsConsumed, out int charsWritten, bool isFinalBlock = true);
    }
}

In addition, there may be some overrides of this new virtual method on derived classes pending discussion\implementation.

cc @GrabYourPitchforks @ahsonkhan

(Method signatures edited by @GrabYourPitchforks on Jul. 16, 2019.)

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 3.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Encodings.Web
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants