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.

Sign up for GitHub

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

Added Span support to Webencoders #334

Closed
wants to merge 24 commits into from
Closed

Added Span support to Webencoders #334

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6f416e3
WebEncodersBenchmarks added
gfoidl Mar 18, 2018
1f7e3ff
A local Buffer<T> added
gfoidl Mar 21, 2018
a03d38d
Decode implemented with Spans
gfoidl Mar 21, 2018
26450cd
Encode implemented with Spans
gfoidl Mar 21, 2018
f32f46c
Little cleanup
gfoidl Mar 21, 2018
194b4ab
Added API
gfoidl Mar 21, 2018
ca91b68
EncodingHelper introduced
gfoidl Mar 23, 2018
acd987c
Optimized EncodingHelper
gfoidl Mar 26, 2018
72a362e
Implementation done for netcoreapp2.1
gfoidl Mar 27, 2018
143a76c
Added buffer-chain
gfoidl Mar 27, 2018
09144d7
Finished multi-targeting
gfoidl Mar 27, 2018
3681f18
Added xml-comments
gfoidl Mar 27, 2018
d309378
Optimized Buffer<T>
gfoidl Mar 28, 2018
007aa1a
Perf improvement
gfoidl Mar 28, 2018
6577241
Removed Buffer<T>
gfoidl Apr 3, 2018
4849dbc
Manual inlining due 'inline exceeds budget'
gfoidl Apr 3, 2018
bdc27dd
Moved private method below public ones, and ordered the public methods
gfoidl Apr 3, 2018
3e7f230
Fixed bugs for netcoreapp2.0
gfoidl Apr 4, 2018
6c2ea6b
Final polishing
gfoidl Apr 4, 2018
7d25346
Renamed EncodingHelper to UrlEncoder
gfoidl Apr 4, 2018
39109e1
Fixed one xml comment
gfoidl Apr 4, 2018
3b56276
PR-feedback
gfoidl Apr 5, 2018
46a5941
Made `SubstituteUrlCharsForEncoding` endianess-aware
gfoidl Apr 9, 2018
2e870a5
Updated GetOperationNotDoneException
gfoidl Apr 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Renamed EncodingHelper to UrlEncoder
  • Loading branch information
@gfoidl
gfoidl committed Jun 18, 2018
commit 7d253463b6c9648f78eaefd200b55d6f702cad66
19 changes: 9 additions & 10 deletions shared/Microsoft.Extensions.WebEncoders.Sources/WebEncoders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,13 @@ public static byte[] Base64UrlDecode(string input, int offset, char[] buffer, in
#if NETCOREAPP2_1
var data = new byte[dataLength];
var base64 = buffer.AsSpan(bufferOffset, base64Len);
EncodingHelper.UrlDecode(input.AsSpan(offset, count), base64);
UrlEncoder.UrlDecode(input.AsSpan(offset, count), base64);
Convert.TryFromBase64Chars(base64, data, out int written);
Debug.Assert(written == dataLength);

return data;
#else
EncodingHelper.UrlDecode(input.AsSpan(offset, count), buffer.AsSpan(bufferOffset, base64Len));
UrlEncoder.UrlDecode(input.AsSpan(offset, count), buffer.AsSpan(bufferOffset, base64Len));
return Convert.FromBase64CharArray(buffer, bufferOffset, base64Len);
#endif
}
Expand Down Expand Up @@ -363,7 +363,7 @@ public static OperationStatus Base64UrlEncode(ReadOnlySpan<byte> data, Span<byte

if (status == OperationStatus.Done || status == OperationStatus.NeedMoreData)
{
bytesWritten = EncodingHelper.UrlEncode(base64Url, bytesWritten);
bytesWritten = UrlEncoder.UrlEncode(base64Url, bytesWritten);
}

return status;
Expand Down Expand Up @@ -494,7 +494,7 @@ private static int Base64UrlDecodeCoreSlow(ReadOnlySpan<char> base64Url, Span<by
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static int Base64UrlDecodeCore(ReadOnlySpan<char> base64Url, Span<byte> base64Bytes, Span<byte> data)
{
EncodingHelper.UrlDecode(base64Url, base64Bytes);
UrlEncoder.UrlDecode(base64Url, base64Bytes);
var status = Base64.DecodeFromUtf8(base64Bytes, data, out int consumed, out int written);

if (status != OperationStatus.Done)
Expand Down Expand Up @@ -531,7 +531,7 @@ private static OperationStatus Base64UrlDecodeCoreSlow(ReadOnlySpan<byte> base64
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static OperationStatus Base64UrlDecodeCore(ReadOnlySpan<byte> base64Url, Span<byte> base64, Span<byte> data, out int consumed, out int written, bool isFinalBlock)
{
EncodingHelper.UrlDecode(base64Url, base64, isFinalBlock);
UrlEncoder.UrlDecode(base64Url, base64, isFinalBlock);
var status = Base64.DecodeFromUtf8(base64, data, out consumed, out written, isFinalBlock);

if (status != OperationStatus.Done && status != OperationStatus.NeedMoreData)
Expand All @@ -555,7 +555,7 @@ private static int Base64UrlEncodeCore(ReadOnlySpan<byte> data, Span<char> base6

Span<char> base64 = stackalloc char[base64Len];
Convert.TryToBase64Chars(data, base64, out int written);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert.TryToBase64Chars is noticeable faster than Base64.EncodeToUtf8. Maybe because base64-encoding and byte -> char is done in one step.

Interestingly on the decoding-methods the Base64-methods are faster.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @ahsonkhan

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last I checked (back in October), the span APIs were on par or faster: dotnet/corefx#24888

@atsushikan, recently did a port of the optimizations to the Convert methods: dotnet/coreclr#17033

There shouldn't be much difference on either now (in the general, non-allocating cases). Can you share details on what type of input you are measuring performance on and maybe a stripped down repro? Otherwise, I will take a deeper look in a couple of days. Thanks for bringing it up.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I had the wrong suspects. Sorry. When running a stripped down sequential repo perf is on par, as noticed by @ahsonkhan

In the vectorized version the difference shows up.

if (Vector.IsHardwareAccelerated && (int*)n >= (int*)Vector<T>.Count)

will make the difference.

For Base64.EncodeToUtf8 T will be byte, so on AVX2 Vector<T>.Count is 32.
For Convert.TryToBase64Chars T will be char (or to be precise ushort), hence Vector<T>.Count is 16.

For a Guid n is 24. So the Convert-version will urlencode vectorized, whereas the Base64-version will run sequential only.

return EncodingHelper.UrlEncode(base64, base64Url);
return UrlEncoder.UrlEncode(base64, base64Url);
}

[MethodImpl(MethodImplOptions.NoInlining)]
Expand All @@ -566,7 +566,7 @@ private static int Base64UrlEncodeCoreSlow(ReadOnlySpan<byte> data, Span<char> b
{
var base64 = new Span<char>(arrayToReturnToPool = ArrayPool<char>.Shared.Rent(base64Len), 0, base64Len);
Convert.TryToBase64Chars(data, base64, out int written);
return EncodingHelper.UrlEncode(base64, base64Url);
return UrlEncoder.UrlEncode(base64, base64Url);
}
finally
{
Expand Down Expand Up @@ -598,7 +598,7 @@ private static int Base64UrlEncodeCore(ReadOnlySpan<byte> data, Span<char> base6
ThrowHelper.ThrowOperationNotDone(status);
}

return EncodingHelper.UrlEncode(base64Bytes, base64Url);
return UrlEncoder.UrlEncode(base64Bytes, base64Url);
#if !NET461
}
finally
Expand Down Expand Up @@ -723,9 +723,8 @@ Exception GetInvalidArgumentsException()
}
}

// internal to make this testable
// TODO: replace IntPtr and (int*) with nuint once available
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nuint isn't avaialbe, so it is a bit messy.

The other workaround we be defining an using alias, depending on an #if for the bitness. On this linked file I didn't know how to do this, so I used the IntPtr-approach.

The TODO comment is to remind me for the change once available.

internal static unsafe class EncodingHelper
internal static unsafe class UrlEncoder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this type UrlCharacterSubstitution or something similar, and change the name of its methods. This type is not actually performing URL encoding, so the name is confusing.

{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void UrlDecode(ReadOnlySpan<byte> urlEncoded, Span<byte> base64, bool isFinalBlock = true)
Expand Down
8 changes: 4 additions & 4 deletions test/Microsoft.Extensions.Internal.Test/WebEncodersTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public void UrlDecode_ReturnsValid_Base64String(string text, string expectedValu
Span<byte> result = stackalloc byte[expectedValue.Length];

// Act
WebEncoders.EncodingHelper.UrlDecode(bytes, result);
WebEncoders.UrlEncoder.UrlDecode(bytes, result);

// Assert
Assert.True(expected.SequenceEqual(result));
Expand All @@ -239,7 +239,7 @@ public void UrlDecode_ReturnsValid_Base64String(string text, string expectedValu
var buffer = new char[expectedValue.Length];

// Act
WebEncoders.EncodingHelper.UrlDecode(text.AsSpan(), buffer);
WebEncoders.UrlEncoder.UrlDecode(text.AsSpan(), buffer);

// Assert
Assert.Equal(expectedValue, new string(buffer));
Expand Down Expand Up @@ -270,7 +270,7 @@ public void UrlEncode_Replaces_UrlEncodableCharacters(string base64EncodedValue,
Encoding.ASCII.GetBytes(expectedValue.AsSpan(), expected);

// Act
var result = WebEncoders.EncodingHelper.UrlEncode(bytes, bytes.Length);
var result = WebEncoders.UrlEncoder.UrlEncode(bytes, bytes.Length);

// Assert
Assert.True(expected.SequenceEqual(bytes.Slice(0, result)));
Expand All @@ -279,7 +279,7 @@ public void UrlEncode_Replaces_UrlEncodableCharacters(string base64EncodedValue,
var buffer = base64EncodedValue.ToCharArray();

// Act
var result = WebEncoders.EncodingHelper.UrlEncode(buffer, buffer);
var result = WebEncoders.UrlEncoder.UrlEncode(buffer, buffer);

// Assert
Assert.Equal(expectedValue.Length, result);
Expand Down