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

Unify int to hexadecimal char conversions #1273

Merged
merged 25 commits into from
Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
1dbff64
Unify int to hexadecimal char conversions
marek-safar Jan 2, 2020
d3f6eb9
More legacy builds fixes
marek-safar Jan 14, 2020
22b1e8a
Get any logging out of CI
marek-safar Jan 15, 2020
463ec5d
Apply Tanner's suggested changes
marek-safar Jan 15, 2020
07666d8
CI logging
marek-safar Jan 15, 2020
0236295
Old netstandard build fixes
marek-safar Jan 19, 2020
168b5c9
Merge remote-tracking branch 'upstream/master' into size-work
marek-safar Jan 20, 2020
8f89e1d
Merge remote-tracking branch 'upstream/master' into size-work
marek-safar Jan 22, 2020
1eda24e
Revert "Apply Tanner's suggested changes"
marek-safar Jan 22, 2020
2e2b28a
Fix inverted logic
marek-safar Jan 24, 2020
d91d965
Update test for side effect of the hex conversion changes
marek-safar Jan 24, 2020
5b0f61a
Fighting some APTC nonsense
marek-safar Jan 24, 2020
af72883
Add one more legacy net define
marek-safar Jan 25, 2020
ddd4b2e
Add another configuration define
marek-safar Jan 27, 2020
2d11acb
Add more defines
marek-safar Jan 27, 2020
e9e623e
Add SecuritySafeCriticalAttribute
marek-safar Jan 27, 2020
5001cbd
Tweak ToString inner loop
marek-safar Jan 27, 2020
aea9539
Merge remote-tracking branch 'upstream/master' into size-work
marek-safar Jan 27, 2020
b8e9287
More of partially trusted stuff
marek-safar Jan 28, 2020
7c14dce
Add assert for legacy builds
marek-safar Jan 28, 2020
0f84e3b
Add licence header
marek-safar Feb 3, 2020
7b08687
Apply second Tanner's version
marek-safar Feb 6, 2020
a2963c2
Replace assert with a code
marek-safar Feb 6, 2020
1116938
Update test name
marek-safar Feb 11, 2020
9a9f789
Merge remote-tracking branch 'upstream/master' into size-work
marek-safar Feb 11, 2020
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
126 changes: 126 additions & 0 deletions src/libraries/Common/src/System/HexConverter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
using System.Runtime.CompilerServices;
marek-safar marked this conversation as resolved.
Show resolved Hide resolved

namespace System
{
internal static class HexConverter
{
public enum Casing : uint
{
// Output [ '0' .. '9' ] and [ 'A' .. 'F' ].
Upper = 0,

// Output [ '0' .. '9' ] and [ 'a' .. 'f' ].
// This works because values in the range [ 0x30 .. 0x39 ] ([ '0' .. '9' ])
// already have the 0x20 bit set, so ORing them with 0x20 is a no-op,
// while outputs in the range [ 0x41 .. 0x46 ] ([ 'A' .. 'F' ])
// don't have the 0x20 bit set, so ORing them maps to
// [ 0x61 .. 0x66 ] ([ 'a' .. 'f' ]), which is what we want.
Lower = 0x2020U,
}
Copy link
Member

Choose a reason for hiding this comment

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

I suspect

static ReadOnlySpan<byte> s_hexEncodeMap => new byte [] { 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 65, 66, 67, 68, 69, 70 };

public static char ToCharUpper(int value) => (char)(s_hexEncodeMap[value & 0xF]); // or even Unsafe.AddByteOffset

should be a bit faster if it's used in hot paths


// We want to pack the incoming byte into a single integer [ 0000 HHHH 0000 LLLL ],
// where HHHH and LLLL are the high and low nibbles of the incoming byte. Then
// subtract this integer from a constant minuend as shown below.
//
// [ 1000 1001 1000 1001 ]
// - [ 0000 HHHH 0000 LLLL ]
// =========================
// [ *YYY **** *ZZZ **** ]
//
// The end result of this is that YYY is 0b000 if HHHH <= 9, and YYY is 0b111 if HHHH >= 10.
// Similarly, ZZZ is 0b000 if LLLL <= 9, and ZZZ is 0b111 if LLLL >= 10.
// (We don't care about the value of asterisked bits.)
//
// To turn a nibble in the range [ 0 .. 9 ] into hex, we calculate hex := nibble + 48 (ascii '0').
// To turn a nibble in the range [ 10 .. 15 ] into hex, we calculate hex := nibble - 10 + 65 (ascii 'A').
// => hex := nibble + 55.
// The difference in the starting ASCII offset is (55 - 48) = 7, depending on whether the nibble is <= 9 or >= 10.
// Since 7 is 0b111, this conveniently matches the YYY or ZZZ value computed during the earlier subtraction.

// The commented out code below is code that directly implements the logic described above.

// uint packedOriginalValues = (((uint)value & 0xF0U) << 4) + ((uint)value & 0x0FU);
// uint difference = 0x8989U - packedOriginalValues;
// uint add7Mask = (difference & 0x7070U) >> 4; // line YYY and ZZZ back up with the packed values
// uint packedResult = packedOriginalValues + add7Mask + 0x3030U /* ascii '0' */;

// The code below is equivalent to the commented out code above but has been tweaked
// to allow codegen to make some extra optimizations.

// The low byte of the packed result contains the hex representation of the incoming byte's low nibble.
// The adjacent byte of the packed result contains the hex representation of the incoming byte's high nibble.

// Finally, write to the output buffer starting with the *highest* index so that codegen can
// elide all but the first bounds check. (This only works if 'startingIndex' is a compile-time constant.)

// The JIT can elide bounds checks if 'startingIndex' is constant and if the caller is
// writing to a span of known length (or the caller has already checked the bounds of the
// furthest access).
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ToBytesBuffer(byte value, Span<byte> buffer, int startingIndex = 0, Casing casing = Casing.Upper)
{
uint difference = (((uint)value & 0xF0U) << 4) + ((uint)value & 0x0FU) - 0x8989U;
uint packedResult = ((((uint)(-(int)difference) & 0x7070U) >> 4) + difference + 0xB9B9U) | (uint)casing;

buffer[startingIndex + 1] = (byte)packedResult;
buffer[startingIndex] = (byte)(packedResult >> 8);
}

#if ALLOW_PARTIALLY_TRUSTED_CALLERS
[System.Security.SecuritySafeCriticalAttribute]
#endif
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ToCharsBuffer(byte value, Span<char> buffer, int startingIndex = 0, Casing casing = Casing.Upper)
{
uint difference = (((uint)value & 0xF0U) << 4) + ((uint)value & 0x0FU) - 0x8989U;
uint packedResult = ((((uint)(-(int)difference) & 0x7070U) >> 4) + difference + 0xB9B9U) | (uint)casing;

buffer[startingIndex + 1] = (char)(packedResult & 0xFF);
buffer[startingIndex] = (char)(packedResult >> 8);
}

#if ALLOW_PARTIALLY_TRUSTED_CALLERS
[System.Security.SecuritySafeCriticalAttribute]
#endif
public static unsafe string ToString(ReadOnlySpan<byte> bytes, Casing casing = Casing.Upper)
{
#if NET45 || NET46 || NET461 || NET462 || NET47 || NET471 || NET472 || NETSTANDARD1_0 || NETSTANDARD1_3 || NETSTANDARD2_0
System.Diagnostics.Debug.Assert(bytes.Length <= 16);
Span<char> result = stackalloc char[bytes.Length * 2];
Copy link
Member

Choose a reason for hiding this comment

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

This seems dangerous. Are all callers limited in how large bytes is?

Copy link
Member

Choose a reason for hiding this comment

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

I see an assert at https://github.com/dotnet/runtime/pull/1273/files#diff-63d8e37549a2478236a87c352cdd1024L1132 that's been removed here. If we're sticking with the stackalloc, we should put that assert back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the assert (not sure how useful that's for legacy builds)

Copy link
Member

@ahsonkhan ahsonkhan Feb 4, 2020

Choose a reason for hiding this comment

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

The bytes.Length <= 16 assert is not guaranteed to be true and this stackalloc could be unsafe. For example, PhysicalAddress takes an unbounded byte array from the user and we pass it here in it's ToString(). That will result in stackoverflow. For lengths > 16 (or some reasonable small constant value, say 128), we should consider doing arraypool.rent or maybe just allocate.

byte[] a = new byte[2048];
var address = new PhysicalAddress(a);
Console.WriteLine(address.ToString());

Copy link
Contributor Author

@marek-safar marek-safar Feb 6, 2020

Choose a reason for hiding this comment

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

It does not matter for this case as the assembly is not built in affected configurations but I changed it anyway as it could show up in System.Security.Cryptography.Pkcs

int pos = 0;
foreach (byte b in bytes)
{
ToCharsBuffer(b, result, pos, casing);
pos += 2;
}
return result.ToString();
#else
fixed (byte* bytesPtr = bytes)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fixed (byte* bytesPtr = bytes)
Debug.Assert(!bytes.IsEmpty);
fixed (byte* bytesPtr = &MemoryMarshal.GetReference(bytes))

Should be a little bit faster. But I don't know if it's worth in contrast to the delegate invocation in string.Create.

{
return string.Create(bytes.Length * 2, (Ptr: (IntPtr)bytesPtr, bytes.Length, casing), (chars, args) =>
{
var ros = new ReadOnlySpan<byte>((byte*)args.Ptr, args.Length);
for (int pos = 0; pos < ros.Length; ++pos)
{
ToCharsBuffer(ros[pos], chars, pos * 2, args.casing);
}
});
}
#endif
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static char ToCharUpper(int value)
{
value &= 0xF;
return (char)(value > 9 ? value - 10 + 'A' : value + '0');
Copy link
Member

Choose a reason for hiding this comment

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

Where did we land on this implementation (are we not going for the ROSpan version)? What was the reason for not going with what @tannergooding suggested here (single branch): #1273 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tanner version is not producing expected output and ROSpan version needs codegen support first (the code can be updated once that lands)

Copy link
Member

Choose a reason for hiding this comment

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

Look like its because the subtraction was off; it needs to subtract '9' + 1. The following passes the unit tests (with the logic all being constant folded):

static void Main(string[] args)
{
    for (int i = 0; i < 0x10; i++)
    {
        Console.WriteLine(ToUpper(i));
    }
}

public static char ToUpper(int value)
{
    value &= 0xF;
    value += '0';

    if (value > '9')
    {
        value += ('A' - ('9' + 1));
    }

    return (char)value;
}

}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static char ToCharLower(int value)
{
value &= 0xF;
return (char)(value > 9 ? value - 10 + 'a' : value + '0');
}
Copy link
Member

Choose a reason for hiding this comment

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

Did you measure this and find it to be faster than:

private static ReadOnlySpan<byte> HexEncodeUpper => new byte [] { (byte)'0', (byte)'1', (byte)'2', (byte)'3', (byte)'4', (byte)'5', (byte)'6', (byte)'7', (byte)'8', (byte)'9', (byte)'A', (byte)'B', (byte)'C', (byte)'D', (byte)'E', (byte)'F' };
private static ReadOnlySpan<byte> HexEncodeLower => new byte [] { (byte)'0', (byte)'1', (byte)'2', (byte)'3', (byte)'4', (byte)'5', (byte)'6', (byte)'7', (byte)'8', (byte)'9', (byte)'a', (byte)'b', (byte)'c', (byte)'d', (byte)'e', (byte)'f' };

public static char ToCharUpper(int value) => (char)HexEncodeUpper[value & 0xF];
public static char ToCharLower(int value) => (char)HexEncodeLower[value & 0xF];

? (Especially once the JIT properly removes the bounds check on this, if it doesn't already.)

Copy link
Member

Choose a reason for hiding this comment

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

(Or using the same tricks/match employed by ToCharsBuffer above.)

Copy link
Member

@EgorBo EgorBo Jan 6, 2020

Choose a reason for hiding this comment

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

@stephentoub

Especially once the JIT properly removes the bounds check on this, if it doesn't already.

It doesn't today but I have a fix for JIT: EgorBo@bcc095e
so it will remove them for things like array[i & c] and array[i % c] if c is less than array.Length.

Copy link
Contributor Author

@marek-safar marek-safar Jan 11, 2020

Choose a reason for hiding this comment

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

Did you measure this and find it to be faster than:

I didn't as these are convenience methods rarely called more than a few times. The Span versions are intended for perf sensitive code.

Also, introducing global memory blob in each assembly which uses this code seems to me contra-productive size-wise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you measure this and find it to be faster than:

Here are the micro-benchmarks for 3 different cases. The sources for them at https://gist.github.com/marek-safar/b26e2bfc9a60a2bd348b3f36d3cb8288

BenchmarkDotNet=v0.12.0, OS=macOS 10.15.2 (19C57) [Darwin 19.2.0]
Intel Core i7-7920HQ CPU 3.10GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=5.0.100-alpha.1.20061.5
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.20.6301, CoreFX 4.700.19.56404), X64 RyuJIT
  DefaultJob : .NET Core 5.0.0 (CoreCLR 5.0.20.6007, CoreFX 5.0.20.6007), X64 RyuJIT

|           Method |      Mean |     Error |    StdDev |
|----------------- |----------:|----------:|----------:|
|   Test_Condition | 0.0257 ns | 0.0310 ns | 0.0290 ns |
|      Test_String | 0.1618 ns | 0.0320 ns | 0.0299 ns |
|         Test_ROS | 0.5877 ns | 0.0473 ns | 0.0506 ns |

Copy link
Member

Choose a reason for hiding this comment

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

I'd interested in numbers from arm64 if you can run it there

Certainly. I'm doing a build for this now and should be able to test sometime after lunch.

Copy link
Member

Choose a reason for hiding this comment

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

From my Samsung Galaxy Book 2:

BenchmarkDotNet=v0.12.0, OS=Windows 10.0.18363
Snapdragon 850 2.96 GHz, 1 CPU, 8 logical and 8 physical cores
.NET Core SDK=5.0.100-alpha1-015515
  [Host]     : .NET Core 5.0.0 (CoreCLR 5.0.19.52501, CoreFX 5.0.19.52503), Arm64 RyuJIT
  DefaultJob : .NET Core 5.0.0 (CoreCLR 5.0.19.52501, CoreFX 5.0.19.52503), Arm64 RyuJIT


|           Method |                Input |     Mean |   Error |  StdDev |
|----------------- |--------------------- |---------:|--------:|--------:|
|    StringVersion | 18446744073709551615 | 147.3 ns | 0.78 ns | 0.73 ns |
| ConditionVersion | 18446744073709551615 | 125.9 ns | 0.32 ns | 0.28 ns |
|       ROSVersion | 18446744073709551615 | 119.1 ns | 0.40 ns | 0.35 ns |

Copy link
Member

Choose a reason for hiding this comment

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

Is the ConditionVersion in this case the one with the 2 branches or the one with your suggestion (and only 1 branch)?

Copy link
Member

Choose a reason for hiding this comment

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

Its thr code from @marek-safar's gist, with no changes

Copy link
Member

Choose a reason for hiding this comment

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

How does the single branch condition perform on Samsung Galaxy Book 2 in comparison to the rest? If it is on-par/better than ROSVersion, then that helps answer which approach we should take (granted, with @EgorBo's changes, the ROSVersion could get better).

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
<None Include="DiagnosticSourceUsersGuide.md" />
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' != 'netstandard1.1'">
<Compile Include="$(CommonPath)System\HexConverter.cs">
<Link>Common\System\HexConverter.cs</Link>
</Compile>
<Compile Include="System\Diagnostics\Activity.cs" />
<Compile Include="System\Diagnostics\DiagnosticSourceActivity.cs" />
<Reference Include="System.Memory" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public string? Id
{
// Convert flags to binary.
Span<char> flagsChars = stackalloc char[2];
ActivityTraceId.ByteToHexDigits(flagsChars, (byte)((~ActivityTraceFlagsIsSet) & _w3CIdFlags));
HexConverter.ToCharsBuffer((byte)((~ActivityTraceFlagsIsSet) & _w3CIdFlags), flagsChars, 0, HexConverter.Casing.Lower);
string id = "00-" + _traceId + "-" + _spanId + "-" + flagsChars.ToString();

Interlocked.CompareExchange(ref _id, id, null);
Expand Down Expand Up @@ -1018,7 +1018,7 @@ public static ActivityTraceId CreateFromBytes(ReadOnlySpan<byte> idData)
if (idData.Length != 16)
throw new ArgumentOutOfRangeException(nameof(idData));

return new ActivityTraceId(SpanToHexString(idData));
return new ActivityTraceId(HexConverter.ToString(idData, HexConverter.Casing.Lower));
}
public static ActivityTraceId CreateFromUtf8String(ReadOnlySpan<byte> idData) => new ActivityTraceId(idData);

Expand Down Expand Up @@ -1097,7 +1097,7 @@ private ActivityTraceId(ReadOnlySpan<byte> idData)
span[1] = BinaryPrimitives.ReverseEndianness(span[1]);
}

_hexString = ActivityTraceId.SpanToHexString(MemoryMarshal.AsBytes(span));
_hexString = HexConverter.ToString(MemoryMarshal.AsBytes(span), HexConverter.Casing.Lower);
}

/// <summary>
Expand All @@ -1120,26 +1120,6 @@ internal static unsafe void SetToRandomBytes(Span<byte> outBytes)
guidBytes.Slice(0, outBytes.Length).CopyTo(outBytes);
}

// CONVERSION binary spans to hex spans, and hex spans to binary spans
/* It would be nice to use generic Hex number conversion routines, but there
* is nothing that is exposed publicly and efficient */
/// <summary>
/// Converts each byte in 'bytes' to hex (thus two characters) and concatenates them
/// and returns the resulting string.
/// </summary>
internal static string SpanToHexString(ReadOnlySpan<byte> bytes)
{
Debug.Assert(bytes.Length <= 16); // We want it to not be very big
Span<char> result = stackalloc char[bytes.Length * 2];
int pos = 0;
foreach (byte b in bytes)
{
result[pos++] = BinaryToHexDigit(b >> 4);
result[pos++] = BinaryToHexDigit(b);
}
return result.ToString();
}

/// <summary>
/// Converts 'idData' which is assumed to be HEX Unicode characters to binary
/// puts it in 'outBytes'
Expand All @@ -1162,20 +1142,6 @@ private static byte HexDigitToBinary(char c)
return (byte)(c - ('a' - 10));
throw new ArgumentOutOfRangeException("idData");
}
private static char BinaryToHexDigit(int val)
{
val &= 0xF;
if (val <= 9)
return (char)('0' + val);
return (char)(('a' - 10) + val);
}

internal static void ByteToHexDigits(Span<char> outChars, byte val)
{
Debug.Assert(outChars.Length == 2);
outChars[0] = BinaryToHexDigit((val >> 4) & 0xF);
outChars[1] = BinaryToHexDigit(val & 0xF);
}

internal static bool IsLowerCaseHexAndNotAllZeros(ReadOnlySpan<char> idData)
{
Expand Down Expand Up @@ -1232,14 +1198,14 @@ public static unsafe ActivitySpanId CreateRandom()
{
ulong id;
ActivityTraceId.SetToRandomBytes(new Span<byte>(&id, sizeof(ulong)));
return new ActivitySpanId(ActivityTraceId.SpanToHexString(new ReadOnlySpan<byte>(&id, sizeof(ulong))));
return new ActivitySpanId(HexConverter.ToString(new ReadOnlySpan<byte>(&id, sizeof(ulong)), HexConverter.Casing.Lower));
}
public static ActivitySpanId CreateFromBytes(ReadOnlySpan<byte> idData)
{
if (idData.Length != 8)
throw new ArgumentOutOfRangeException(nameof(idData));

return new ActivitySpanId(ActivityTraceId.SpanToHexString(idData));
return new ActivitySpanId(HexConverter.ToString(idData, HexConverter.Casing.Lower));
}
public static ActivitySpanId CreateFromUtf8String(ReadOnlySpan<byte> idData) => new ActivitySpanId(idData);

Expand Down Expand Up @@ -1307,7 +1273,7 @@ private unsafe ActivitySpanId(ReadOnlySpan<byte> idData)
id = BinaryPrimitives.ReverseEndianness(id);
}

_hexString = ActivityTraceId.SpanToHexString(new ReadOnlySpan<byte>(&id, sizeof(ulong)));
_hexString = HexConverter.ToString(new ReadOnlySpan<byte>(&id, sizeof(ulong)), HexConverter.Casing.Lower);
}

/// <summary>
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@
<Compile Include="$(CommonPath)System\Text\SimpleRegex.cs">
<Link>Common\System\Text\SimpleRegex.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\HexConverter.cs">
<Link>Common\System\HexConverter.cs</Link>
</Compile>
</ItemGroup>
<!-- SocketsHttpHandler implementation -->
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ internal static class HeaderUtilities
// Validator
internal static readonly Action<HttpHeaderValueCollection<string>, string> TokenValidator = ValidateToken;

private static readonly char[] s_hexUpperChars = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };

internal static void SetQuality(ObjectCollection<NameValueHeaderValue> parameters, double? value)
{
Debug.Assert(parameters != null);
Expand Down Expand Up @@ -125,8 +123,8 @@ private static void AddHexEscaped(byte c, StringBuilder destination)
Debug.Assert(destination != null);

destination.Append('%');
destination.Append(s_hexUpperChars[(c & 0xf0) >> 4]);
destination.Append(s_hexUpperChars[c & 0xf]);
destination.Append(HexConverter.ToCharUpper(c >> 4));
destination.Append(HexConverter.ToCharUpper(c));
}

internal static double? GetQuality(ObjectCollection<NameValueHeaderValue> parameters)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
<DefaultReferenceExclusions Include="System.Net.Mail" />
</ItemGroup>
<ItemGroup>
<Compile Include="$(CommonPath)System\HexConverter.cs">
<Link>Common\System\HexConverter.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\NotImplemented.cs">
<Link>ProductionCode\Common\System\NotImplemented.cs</Link>
</Compile>
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Net.Mail/src/System.Net.Mail.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@
<Compile Include="$(CommonPath)System\Net\Security\NetEventSource.Security.cs">
<Link>Common\System\Net\Security\NetEventSource.Security.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\HexConverter.cs">
<Link>Common\System\HexConverter.cs</Link>
</Compile>
</ItemGroup>
<!-- Unix specific files -->
<ItemGroup Condition="'$(TargetsUnix)'=='true'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ internal class QEncodedStream : DelegatedStream, IEncodableStream
255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, // F
};

//bytes that correspond to the hex char representations in ASCII (0-9, A-F)
private static readonly byte[] s_hexEncodeMap = new byte[] { 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 65, 66, 67, 68, 69, 70 };

private ReadStateInfo _readState;
private readonly WriteStateInfoBase _writeState;

Expand Down Expand Up @@ -248,9 +245,9 @@ public int EncodeBytes(byte[] buffer, int offset, int count)
//append an = to indicate an encoded character
WriteState.Append((byte)'=');
//shift 4 to get the first four bytes only and look up the hex digit
WriteState.Append(s_hexEncodeMap[buffer[cur] >> 4]);
WriteState.Append((byte)HexConverter.ToCharUpper(buffer[cur] >> 4));
//clear the first four bytes to get the last four and look up the hex digit
WriteState.Append(s_hexEncodeMap[buffer[cur] & 0xF]);
WriteState.Append((byte)HexConverter.ToCharUpper(buffer[cur]));
}
}
WriteState.AppendFooter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@
<Compile Include="$(CommonPath)System\Net\Security\NetEventSource.Security.cs">
<Link>Common\System\Net\Security\NetEventSource.Security.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\HexConverter.cs">
<Link>Common\System\HexConverter.cs</Link>
</Compile>
</ItemGroup>
<!-- Unix specific files -->
<ItemGroup Condition="'$(TargetsUnix)'=='true'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@
<Compile Include="$(CommonPath)System\Net\NetworkInformation\NetworkInformationException.cs">
<Link>Common\System\Net\NetworkInformation\NetworkInformationException.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\HexConverter.cs">
<Link>Common\System\HexConverter.cs</Link>
</Compile>
</ItemGroup>
<ItemGroup Condition="'$(TargetsWindows)' == 'true'">
<!-- Logging -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,7 @@ public override bool Equals(object comparand)

public override string ToString()
{
return string.Create(_address.Length * 2, _address, (span, addr) =>
{
int p = 0;
foreach (byte value in addr)
{
byte upper = (byte)(value >> 4), lower = (byte)(value & 0xF);
span[p++] = (char)(upper + (upper < 10 ? '0' : 'A' - 10));
span[p++] = (char)(lower + (lower < 10 ? '0' : 'A' - 10));
}
});
return HexConverter.ToString(_address.AsSpan(), HexConverter.Casing.Upper);
}

public byte[] GetAddressBytes()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public void TryParseSpan_Invalid_ReturnsFalse(string address)
public void ToString_NullAddress_NullReferenceException()
{
var pa = new PhysicalAddress(null);
Assert.Throws<NullReferenceException>(() => pa.ToString());
Assert.Equal("", pa.ToString());
marek-safar marked this conversation as resolved.
Show resolved Hide resolved
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@
<Compile Include="$(CommonPath)System\Runtime\CompilerServices\PreserveDependencyAttribute.cs">
<Link>Common\System\Runtime\CompilerServices\PreserveDependencyAttribute.cs</Link>
</Compile>
<Compile Include="$(CommonPath)System\HexConverter.cs">
<Link>Common\System\HexConverter.cs</Link>
</Compile>
<!-- Logging -->
<Compile Include="$(CommonPath)System\Net\Logging\NetEventSource.Common.cs">
<Link>Common\System\Net\Logging\NetEventSource.Common.cs</Link>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,22 +279,18 @@ private static void AppendSections(ushort[] address, int fromInclusive, int toEx
}

/// <summary>Appends a number as hexadecimal (without the leading "0x") to the StringBuilder.</summary>
private static unsafe void AppendHex(ushort value, StringBuilder buffer)
private static void AppendHex(ushort value, StringBuilder buffer)
{
const int MaxLength = sizeof(ushort) * 2; // two hex chars per byte
char* chars = stackalloc char[MaxLength];
int pos = MaxLength;
if ((value & 0xF000) != 0)
buffer.Append(HexConverter.ToCharLower(value >> 12));

do
{
int rem = value % 16;
value /= 16;
chars[--pos] = rem < 10 ? (char)('0' + rem) : (char)('a' + (rem - 10));
Debug.Assert(pos >= 0);
}
while (value != 0);
if ((value & 0xFF00) != 0)
buffer.Append(HexConverter.ToCharLower(value >> 8));

if ((value & 0xFFF0) != 0)
buffer.Append(HexConverter.ToCharLower(value >> 4));

buffer.Append(chars + pos, MaxLength - pos);
buffer.Append(HexConverter.ToCharLower(value));
}

/// <summary>Extracts the IPv4 address from the end of the IPv6 address byte array.</summary>
Expand Down
Loading