Add Span support to WebEncoders

[BrennanConroy]

We should add span overloads to the WebEncoders class, and also use the Base64Encoder/Decoder classes from CoreFX
https://github.com/dotnet/corefx/blob/master/src/System.Memory/src/System/Buffers/Text/Base64Encoder.cs

[davidfowl]

Hey @gfoidl, care to take a look at this one? Seems up your alley 😄

[gfoidl]

I'll have a look and try to route it down this alley 😄

[davidfowl]

@gfoidl some quick ideas:

• Add some benchmarks so we have a performance baseline
• Write the core API against Span<char> and forward all the existing API calls to that version
• For the APIs that return a string, we can compute the destination length and use the new string.Create method in .NET Core 2.1 (requires an ifdef but it's worth it).
• Add a Span<byte> overload for UTF8 data specifically
• Add an overload that formats into a destination Span<byte> so we can use stackalloc

/cc @KrzysztofCwalina @ahsonkhan

[gfoidl]

Thanks for the pointers. I'll try to make progress over the weekend, today is at bit busy...

To be sure:
https://github.com/aspnet/Common/blob/dev/shared/Microsoft.Extensions.WebEncoders.Sources/WebEncoders.cs is meant?
https://github.com/aspnet/HtmlAbstractions/tree/dev/src/Microsoft.Extensions.WebEncoders is unrelated to this change

[BrennanConroy]

Correct

[KrzysztofCwalina]

It would be also good if the encoders supported encoding/decoding UTF8 buffers.
Also, the APIs should support buffer chains, just like Base64 encoder/decoder does: https://github.com/dotnet/corefx/blob/master/src/System.Memory/ref/System.Memory.cs#L457

cc: @GrabYourPitchforks

[GrabYourPitchforks]

We need to give the encoders an overhaul anyway when the UTF8 support comes online since the current implementation is optimized for UTF16. I've been prototyping this recently and have made decent progress.

[gfoidl]

So should I wait for @GrabYourPitchforks' progress (do you have any rough timelline on that?), then overhaul the whole encoders-class or start implementing this proposed change and overhaul it later on?

[davidfowl]

No don’t wait for that. This is something we’d want for 2.1. At a very minimum we want to be able to pass a Span<byte> in and get back a string

[gfoidl]

OK -- I'll start with.

[gfoidl]

While implementing some questions arose:

API additions

Additions to the current API would be:

public static byte[] Base64UrlDecode(ReadOnlySpan<char> input);
public static int Base64UrlDecode(ReadOnlySpan<char> input, Span<byte> output);		// returns bytes written

public static string Base64UrlEncode(ReadOnlySpan<byte> input);
public static int Base64UrlEncode(ReadOnlySpan<byte> input, Span<char> output);		// returns chars written

#if NETCOREAPP2_1		// UTF-8 variants
public static OperationStatus Base64UrlDecode(ReadOnlySpan<byte> utf8, Span<byte> output, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
public static OperationStatus Base64UrlEncode(ReadOnlySpan<byte> input, Span<byte> utf8, out int bytesConsumed, out int bytesWritten, bool isFinalBlock = true);
#endif

Are these OK?

---

• Except the UTF-8 variants they are already prototyped. Needs some more perf-tuning and XML-comments
• BTW: Should / can I bring the UTF-8 variants in a PR of its own? The PRs would be clearer and Span-overloads could get merged earlier.

---

String.Create

Edit: it's quite trivial to compute the number of padding-chars, so no problem.

string.Create<T> is a great addition, but it requires that the length of the constructed string is known in advance.

we can compute the destination length

Maybe we can, but I can't 😉
The "Encode"-methods are concerned. The workflow is:
bytes -> base64 -> Url-Encode -> string

I can compute the minimum length required for the base64 char-buffer. (count + 2) / 3 * 4.
Without duplication of the base64-algorithm I don't know the exact length.
Then I don't know in advance how many padding chars = there will be, which will be removed in the url-encoding step.

So in the prototype I ended up with

return new String(output);		// output is a Span<char>

So maybe I miss something right now, or ... you can give me a hint how to do it.

[GrabYourPitchforks]

@gfoidl Sorry, I should've clarified earlier. The WebEncoders (HTML, JavaScript, etc.) need to be overhauled when UTF-8 support comes online, and those are the only ones I've looked at in detail. Other encoders like base64url encoding are up for grabs as far as I know.

[gfoidl]

@davidfowl I just noticed the "up for grabs" label. I've started working on this, just some perf tweaks needs to be done. Hopefully I can submit a PR within the next two days. Can you assign me this issue?

[analogrelay]

@gfoidl only members of the repo can be assigned, but if you're still interested in submitting a PR we'd welcome it!

[BrennanConroy]

Here is the previous attempt #338

I do really want at least part of this in so we can get rid of an allocation in SignalR 😄

[gfoidl]

want at least part of this in

Tell which part you want, then I can submit a (new) PR. Also if you want to have it simd-based or pure scalar code.

[BrennanConroy]

I want WebEncoders.Base64UrlEncode to accept a span.

Here is the affected code
https://github.com/aspnet/AspNetCore/blob/7ee7f5ddeea480e4046f042af8d58182e7117d1b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs#L110-L113

[gfoidl]

I'll have a look and submit a PR.

[gfoidl]

I took a look 😉 -> dotnet/aspnetcore#11047

[analogrelay]

Appears to have been done.