Skip to content

Commit

Permalink
Refactoring with .NET 5 Unsafe APIs (#3510)
Browse files Browse the repository at this point in the history
## Close #3509 
<!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. -->

<!-- Add a brief overview here of the feature/bug & fix. -->

## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->

- Refactoring (no functional changes, no api changes)
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->


## What is the current behavior?
<!-- Please describe the current behavior that you are modifying, or link to a relevant issue. -->
There are a number of usages of `Unsafe.AsRef<T>(null)` and similar that are verbose.

## What is the new behavior?
<!-- Describe how was this issue resolved or changed? -->
These occurrences are replaced using the new `Unsafe.NullRef<T>()` and `Unsafe.IsNullRef<T>(...)` APIs.

## Notes

Leaving this as draft as the new `System.Runtime.CompilerServices.Unsafe` package is still in release candidate.

## PR Checklist

Please check if your PR fulfills the following requirements:

- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->~~
- [ ] ~~Sample in sample app has been added / updated (for bug fixes / features)~~
    - [ ] ~~Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [X] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes
  • Loading branch information
msftbot[bot] authored Nov 10, 2020
2 parents 66be202 + 33aadff commit 92836d5
Show file tree
Hide file tree
Showing 14 changed files with 55 additions and 60 deletions.
8 changes: 4 additions & 4 deletions Microsoft.Toolkit.HighPerformance/Box{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public static Box<T> GetFrom(object obj)
ThrowInvalidCastExceptionForGetFrom();
}

return Unsafe.As<Box<T>>(obj);
return Unsafe.As<Box<T>>(obj)!;
}

/// <summary>
Expand All @@ -94,7 +94,7 @@ public static Box<T> GetFrom(object obj)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Box<T> DangerousGetFrom(object obj)
{
return Unsafe.As<Box<T>>(obj);
return Unsafe.As<Box<T>>(obj)!;
}

/// <summary>
Expand All @@ -108,7 +108,7 @@ public static bool TryGetFrom(object obj, [NotNullWhen(true)] out Box<T>? box)
{
if (obj.GetType() == typeof(T))
{
box = Unsafe.As<Box<T>>(obj);
box = Unsafe.As<Box<T>>(obj)!;

return true;
}
Expand Down Expand Up @@ -145,7 +145,7 @@ public static implicit operator Box<T>(T value)
// manually be implemented in the Box<T> type. For instance, boxing a float
// and calling ToString() on it directly, on its boxed object or on a Box<T>
// reference retrieved from it will produce the same result in all cases.
return Unsafe.As<Box<T>>(value);
return Unsafe.As<Box<T>>(value)!;
}

/// <inheritdoc/>
Expand Down
20 changes: 10 additions & 10 deletions Microsoft.Toolkit.HighPerformance/Buffers/StringPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -422,11 +422,11 @@ public object SyncRoot
/// <param name="value">The input <see cref="string"/> instance to cache.</param>
/// <param name="hashcode">The precomputed hashcode for <paramref name="value"/>.</param>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public unsafe void Add(string value, int hashcode)
public void Add(string value, int hashcode)
{
ref string target = ref TryGet(value.AsSpan(), hashcode);

if (Unsafe.AreSame(ref target, ref Unsafe.AsRef<string>(null)))
if (Unsafe.IsNullRef(ref target))
{
Insert(value, hashcode);
}
Expand All @@ -443,11 +443,11 @@ public unsafe void Add(string value, int hashcode)
/// <param name="hashcode">The precomputed hashcode for <paramref name="value"/>.</param>
/// <returns>A <see cref="string"/> instance with the contents of <paramref name="value"/>.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public unsafe string GetOrAdd(string value, int hashcode)
public string GetOrAdd(string value, int hashcode)
{
ref string result = ref TryGet(value.AsSpan(), hashcode);

if (!Unsafe.AreSame(ref result, ref Unsafe.AsRef<string>(null)))
if (!Unsafe.IsNullRef(ref result))
{
return result;
}
Expand All @@ -464,11 +464,11 @@ public unsafe string GetOrAdd(string value, int hashcode)
/// <param name="hashcode">The precomputed hashcode for <paramref name="span"/>.</param>
/// <returns>A <see cref="string"/> instance with the contents of <paramref name="span"/>, cached if possible.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public unsafe string GetOrAdd(ReadOnlySpan<char> span, int hashcode)
public string GetOrAdd(ReadOnlySpan<char> span, int hashcode)
{
ref string result = ref TryGet(span, hashcode);

if (!Unsafe.AreSame(ref result, ref Unsafe.AsRef<string>(null)))
if (!Unsafe.IsNullRef(ref result))
{
return result;
}
Expand All @@ -488,11 +488,11 @@ public unsafe string GetOrAdd(ReadOnlySpan<char> span, int hashcode)
/// <param name="value">The resulting cached <see cref="string"/> instance, if present</param>
/// <returns>Whether or not the target <see cref="string"/> instance was found.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public unsafe bool TryGet(ReadOnlySpan<char> span, int hashcode, [NotNullWhen(true)] out string? value)
public bool TryGet(ReadOnlySpan<char> span, int hashcode, [NotNullWhen(true)] out string? value)
{
ref string result = ref TryGet(span, hashcode);

if (!Unsafe.AreSame(ref result, ref Unsafe.AsRef<string>(null)))
if (!Unsafe.IsNullRef(ref result))
{
value = result;

Expand Down Expand Up @@ -527,7 +527,7 @@ public void Reset()
private unsafe ref string TryGet(ReadOnlySpan<char> span, int hashcode)
{
ref MapEntry mapEntriesRef = ref this.mapEntries.DangerousGetReference();
ref MapEntry entry = ref Unsafe.AsRef<MapEntry>(null);
ref MapEntry entry = ref Unsafe.NullRef<MapEntry>();
int
length = this.buckets.Length,
bucketIndex = hashcode & (length - 1);
Expand All @@ -547,7 +547,7 @@ private unsafe ref string TryGet(ReadOnlySpan<char> span, int hashcode)
}
}

return ref Unsafe.AsRef<string>(null);
return ref Unsafe.NullRef<string>();
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public static partial class ArrayExtensions
public static ref T DangerousGetReference<T>(this T[] array)
{
#if NETCORE_RUNTIME
var arrayData = Unsafe.As<RawArrayData>(array);
var arrayData = Unsafe.As<RawArrayData>(array)!;
ref T r0 = ref Unsafe.As<byte, T>(ref arrayData.Data);

return ref r0;
Expand All @@ -55,7 +55,7 @@ public static ref T DangerousGetReference<T>(this T[] array)
public static ref T DangerousGetReferenceAt<T>(this T[] array, int i)
{
#if NETCORE_RUNTIME
var arrayData = Unsafe.As<RawArrayData>(array);
var arrayData = Unsafe.As<RawArrayData>(array)!;
ref T r0 = ref Unsafe.As<byte, T>(ref arrayData.Data);
ref T ri = ref Unsafe.Add(ref r0, (nint)(uint)i);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static partial class ArrayExtensions
public static ref T DangerousGetReference<T>(this T[,] array)
{
#if NETCORE_RUNTIME
var arrayData = Unsafe.As<RawArray2DData>(array);
var arrayData = Unsafe.As<RawArray2DData>(array)!;
ref T r0 = ref Unsafe.As<byte, T>(ref arrayData.Data);

return ref r0;
Expand Down Expand Up @@ -63,7 +63,7 @@ public static ref T DangerousGetReference<T>(this T[,] array)
public static ref T DangerousGetReferenceAt<T>(this T[,] array, int i, int j)
{
#if NETCORE_RUNTIME
var arrayData = Unsafe.As<RawArray2DData>(array);
var arrayData = Unsafe.As<RawArray2DData>(array)!;
nint offset = ((nint)(uint)i * (nint)(uint)arrayData.Width) + (nint)(uint)j;
ref T r0 = ref Unsafe.As<byte, T>(ref arrayData.Data);
ref T ri = ref Unsafe.Add(ref r0, offset);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static partial class ArrayExtensions
public static ref T DangerousGetReference<T>(this T[,,] array)
{
#if NETCORE_RUNTIME
var arrayData = Unsafe.As<RawArray3DData>(array);
var arrayData = Unsafe.As<RawArray3DData>(array)!;
ref T r0 = ref Unsafe.As<byte, T>(ref arrayData.Data);

return ref r0;
Expand Down Expand Up @@ -63,7 +63,7 @@ public static ref T DangerousGetReference<T>(this T[,,] array)
public static ref T DangerousGetReferenceAt<T>(this T[,,] array, int i, int j, int k)
{
#if NETCORE_RUNTIME
var arrayData = Unsafe.As<RawArray3DData>(array);
var arrayData = Unsafe.As<RawArray3DData>(array)!;
nint offset =
((nint)(uint)i * (nint)(uint)arrayData.Height * (nint)(uint)arrayData.Width) +
((nint)(uint)j * (nint)(uint)arrayData.Width) + (nint)(uint)k;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public static class ObjectExtensions
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static IntPtr DangerousGetObjectDataByteOffset<T>(this object obj, ref T data)
{
var rawObj = Unsafe.As<RawObjectData>(obj);
var rawObj = Unsafe.As<RawObjectData>(obj)!;
ref byte r0 = ref rawObj.Data;
ref byte r1 = ref Unsafe.As<T, byte>(ref data);

Expand All @@ -55,7 +55,7 @@ public static IntPtr DangerousGetObjectDataByteOffset<T>(this object obj, ref T
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static ref T DangerousGetObjectDataReferenceAt<T>(this object obj, IntPtr offset)
{
var rawObj = Unsafe.As<RawObjectData>(obj);
var rawObj = Unsafe.As<RawObjectData>(obj)!;
ref byte r0 = ref rawObj.Data;
ref byte r1 = ref Unsafe.AddByteOffset(ref r0, offset);
ref T r2 = ref Unsafe.As<byte, T>(ref r1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public static ref char DangerousGetReference(this string text)
#if NETCOREAPP3_1
return ref Unsafe.AsRef(text.GetPinnableReference());
#elif NETCOREAPP2_1
var stringData = Unsafe.As<RawStringData>(text);
var stringData = Unsafe.As<RawStringData>(text)!;

return ref stringData.Data;
#else
Expand All @@ -53,7 +53,7 @@ public static ref char DangerousGetReferenceAt(this string text, int i)
#if NETCOREAPP3_1
ref char r0 = ref Unsafe.AsRef(text.GetPinnableReference());
#elif NETCOREAPP2_1
ref char r0 = ref Unsafe.As<RawStringData>(text).Data;
ref char r0 = ref Unsafe.As<RawStringData>(text)!.Data;
#else
ref char r0 = ref MemoryMarshal.GetReference(text.AsSpan());
#endif
Expand Down
11 changes: 4 additions & 7 deletions Microsoft.Toolkit.HighPerformance/Memory/Memory2D{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ public bool TryGetMemory(out Memory<T> memory)
}
else if (typeof(T) == typeof(char) && this.instance.GetType() == typeof(string))
{
string text = Unsafe.As<string>(this.instance);
string text = Unsafe.As<string>(this.instance)!;
int index = text.AsSpan().IndexOf(in text.DangerousGetObjectDataReferenceAt<char>(this.offset));
ReadOnlyMemory<char> temp = text.AsMemory(index, (int)Length);

Expand All @@ -786,16 +786,13 @@ public bool TryGetMemory(out Memory<T> memory)
}
else if (this.instance is MemoryManager<T> memoryManager)
{
unsafe
{
// If the object is a MemoryManager<T>, just slice it as needed
memory = memoryManager.Memory.Slice((int)(void*)this.offset, this.height * this.width);
}
// If the object is a MemoryManager<T>, just slice it as needed
memory = memoryManager.Memory.Slice((int)(nint)this.offset, this.height * this.width);
}
else if (this.instance.GetType() == typeof(T[]))
{
// If it's a T[] array, also handle the initial offset
T[] array = Unsafe.As<T[]>(this.instance);
T[] array = Unsafe.As<T[]>(this.instance)!;
int index = array.AsSpan().IndexOf(ref array.DangerousGetObjectDataReferenceAt<T>(this.offset));

memory = array.AsMemory(index, this.height * this.width);
Expand Down
11 changes: 4 additions & 7 deletions Microsoft.Toolkit.HighPerformance/Memory/ReadOnlyMemory2D{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -794,24 +794,21 @@ public bool TryGetMemory(out ReadOnlyMemory<T> memory)
// difference between the start of the Span<char> (which directly wraps just the actual character data
// within the string), and the input reference, which we can get from the byte offset in use. The result
// is the character index which we can use to create the final Memory<char> instance.
string text = Unsafe.As<string>(this.instance);
string text = Unsafe.As<string>(this.instance)!;
int index = text.AsSpan().IndexOf(in text.DangerousGetObjectDataReferenceAt<char>(this.offset));
ReadOnlyMemory<char> temp = text.AsMemory(index, (int)Length);

memory = Unsafe.As<ReadOnlyMemory<char>, ReadOnlyMemory<T>>(ref temp);
}
else if (this.instance is MemoryManager<T> memoryManager)
{
unsafe
{
// If the object is a MemoryManager<T>, just slice it as needed
memory = memoryManager.Memory.Slice((int)(void*)this.offset, this.height * this.width);
}
// If the object is a MemoryManager<T>, just slice it as needed
memory = memoryManager.Memory.Slice((int)(nint)this.offset, this.height * this.width);
}
else if (this.instance.GetType() == typeof(T[]))
{
// If it's a T[] array, also handle the initial offset
T[] array = Unsafe.As<T[]>(this.instance);
T[] array = Unsafe.As<T[]>(this.instance)!;
int index = array.AsSpan().IndexOf(ref array.DangerousGetObjectDataReferenceAt<T>(this.offset));

memory = array.AsMemory(index, this.height * this.width);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<PackageReference Include="System.Numerics.Vectors" Version="4.5.0" />
<PackageReference Include="System.Threading.Tasks.Extensions" Version="4.5.4" />
<PackageReference Include="System.Threading.Tasks.Parallel" Version="4.3.0" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.7.1" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
</ItemGroup>
</When>
<When Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
Expand All @@ -51,14 +51,14 @@
<PackageReference Include="Microsoft.Bcl.HashCode" Version="1.1.0" />
<PackageReference Include="System.Memory" Version="4.5.4" />
<PackageReference Include="System.Threading.Tasks.Extensions" Version="4.5.4" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.7.1" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
</ItemGroup>
</When>
<When Condition=" '$(TargetFramework)' == 'netstandard2.1' ">
<ItemGroup>

<!-- .NET Standard 2.1 doesn't have the Unsafe type -->
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.7.1" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
</ItemGroup>
<PropertyGroup>

Expand All @@ -76,6 +76,9 @@
</PropertyGroup>
</When>
<When Condition=" '$(TargetFramework)' == 'netcoreapp3.1' ">
<ItemGroup>
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
</ItemGroup>
<PropertyGroup>

<!-- NETCORE_RUNTIME: to avoid issues with APIs that assume a specific memory layout, we define a
Expand All @@ -89,7 +92,7 @@
</When>
<When Condition=" '$(TargetFramework)' == 'netcoreapp2.1' ">
<ItemGroup>
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.7.1" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
</ItemGroup>
<PropertyGroup>
<DefineConstants>SPAN_RUNTIME_SUPPORT;NETCORE_RUNTIME</DefineConstants>
Expand Down
28 changes: 13 additions & 15 deletions Microsoft.Toolkit.Uwp/Helpers/ColorHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Globalization;
using System.Reflection;
using Microsoft.Toolkit.Diagnostics;
using Windows.UI;
using Color = Windows.UI.Color;

Expand All @@ -22,10 +24,7 @@ public static class ColorHelper
/// <returns>The created <see cref="Color"/>.</returns>
public static Color ToColor(this string colorString)
{
if (string.IsNullOrEmpty(colorString))
{
throw new ArgumentException(nameof(colorString));
}
Guard.IsNotNullOrEmpty(colorString, nameof(colorString));

if (colorString[0] == '#')
{
Expand Down Expand Up @@ -80,8 +79,7 @@ public static Color ToColor(this string colorString)
return Color.FromArgb(255, r, g, b);
}

default:
throw new FormatException(string.Format("The {0} string passed in the colorString argument is not a recognized Color format.", colorString));
default: return ThrowHelper.ThrowFormatException<Color>("The string passed in the colorString argument is not a recognized Color format.");
}
}

Expand All @@ -91,24 +89,24 @@ public static Color ToColor(this string colorString)

if (values.Length == 4)
{
var scA = double.Parse(values[0].Substring(3));
var scR = double.Parse(values[1]);
var scG = double.Parse(values[2]);
var scB = double.Parse(values[3]);
var scA = double.Parse(values[0].Substring(3), CultureInfo.InvariantCulture);
var scR = double.Parse(values[1], CultureInfo.InvariantCulture);
var scG = double.Parse(values[2], CultureInfo.InvariantCulture);
var scB = double.Parse(values[3], CultureInfo.InvariantCulture);

return Color.FromArgb((byte)(scA * 255), (byte)(scR * 255), (byte)(scG * 255), (byte)(scB * 255));
}

if (values.Length == 3)
{
var scR = double.Parse(values[0].Substring(3));
var scG = double.Parse(values[1]);
var scB = double.Parse(values[2]);
var scR = double.Parse(values[0].Substring(3), CultureInfo.InvariantCulture);
var scG = double.Parse(values[1], CultureInfo.InvariantCulture);
var scB = double.Parse(values[2], CultureInfo.InvariantCulture);

return Color.FromArgb(255, (byte)(scR * 255), (byte)(scG * 255), (byte)(scB * 255));
}

throw new FormatException(string.Format("The {0} string passed in the colorString argument is not a recognized Color format (sc#[scA,]scR,scG,scB).", colorString));
return ThrowHelper.ThrowFormatException<Color>("The string passed in the colorString argument is not a recognized Color format (sc#[scA,]scR,scG,scB).");
}

var prop = typeof(Colors).GetTypeInfo().GetDeclaredProperty(colorString);
Expand All @@ -118,7 +116,7 @@ public static Color ToColor(this string colorString)
return (Color)prop.GetValue(null);
}

throw new FormatException(string.Format("The {0} string passed in the colorString argument is not a recognized Color.", colorString));
return ThrowHelper.ThrowFormatException<Color>("The string passed in the colorString argument is not a recognized Color.");
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

<!-- .NET Core 2.1 doesn't have the Unsafe type -->
<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp2.1' ">
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.7.1" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
<Version>1.4.0</Version>
</PackageReference>
<PackageReference Include="System.Runtime.CompilerServices.Unsafe">
<Version>4.7.1</Version>
<Version>5.0.0</Version>
</PackageReference>
</ItemGroup>
<ItemGroup>
Expand Down
Loading

0 comments on commit 92836d5

Please sign in to comment.