2D System.Memory-like primitives (Span2D<T>, Memory2D<T>) #3353

Sergio0694 · 2020-06-17T14:57:46Z

"If I have a 3D array, I want to take one of the slices of the 2D array out of it as a reference...?"

And this is the answer 🚀

PR Type

What kind of change does this PR introduce?

Feature

What is the current behavior?

There is no proper way to pass around a 2D memory area in a format that's easy to handle and somewhat similar to how the other System.Memory APIs work (ie. Span<T>, Memory<T>). This is regardless of whether or not the wrapped memory region is contiguous in memory or not.

What is the new behavior?

This PR adds a number of new types and extensions to solve this issue, specifically:

Span2D<T>
ReadOnlySpan2D<T>
Memory2D<T>
ReadOnlyMemory2D<T>

And a series of new extensions for 3D arrays as well.
Note that all these 2D types can be used with arrays of any type, eg. they can both be used to wrap a 2D memory region from a 2D array, a 2D slice for a 3D array, or even just to represent a 1D array as a 2D memory region to make it easier to properly access elements by their spatial coordinates.

NOTE: as mentioned above, the wrapped 2D area does not need to be contiguous! See comments in Span2D<T> to see how this is handled. Having this feature gives users much more freedom and flexibility when working with arbitrary arrays (or unmanaged memory too).

API surface (summary)

namespace Microsoft.Toolkit.HighPerformance.Enumerables
{
    public readonly ref struct RefEnumerable<T>
    {
        public readonly Enumerator GetEnumerator();
        public readonly void Clear();
        public readonly void CopyTo(RefEnumerable<T> destination);
        public readonly bool TryCopyTo(RefEnumerable<T> destination);
        public readonly void CopyTo(Span<T> destination);
        public readonly bool TryCopyTo(Span<T> destination);
        public readonly void Fill(T value);
        public readonly T[] ToArray();

        public ref struct Enumerator
        {
            public readonly ref T Current { get; }

            public bool MoveNext();
        }
    }

    public readonly ref struct ReadOnlyRefEnumerable<T>
    {
        public readonly Enumerator GetEnumerator();
        public readonly void CopyTo(RefEnumerable<T> destination);
        public readonly bool TryCopyTo(RefEnumerable<T> destination);
        public readonly void CopyTo(Span<T> destination);
        public readonly bool TryCopyTo(Span<T> destination);
        public readonly T[] ToArray();

        public static implicit operator ReadOnlyRefEnumerable<T>(RefEnumerable<T> enumerable);

        public ref struct Enumerator
        {
            public readonly ref readonly T Current { get; }

            public bool MoveNext();
        }
    }
}

namespace Microsoft.Toolkit.HighPerformance.Extensions
{
    public static class ArrayExtensions
    {
        public static bool IsCovariant<T>(this T[] array);

        public static RefEnumerable<T> GetRow<T>(this T[,] array, int row);
        public static RefEnumerable<T> GetColumn<T>(this T[,] array, int column);
        public static Span<T> GetRowSpan<T>(this T[,] array, int row);
        public static Memory<T> GetRowMemory<T>(this T[,] array, int row);
        public static Memory<T> AsMemory<T>(this T[,]? array);
        public static Span2D<T> AsSpan2D<T>(this T[,]? array);
        public static Span2D<T> AsSpan2D<T>(this T[,]? array, int row, int column, int height, int width);
        public static Memory2D<T> AsMemory2D<T>(this T[,]? array);
        public static Memory2D<T> AsMemory2D<T>(this T[,]? array, int row, int column, int height, int width);
        public static bool IsCovariant<T>(this T[,] array);

        public static ref T DangerousGetReference<T>(this T[,,] array);
        public static ref T DangerousGetReferenceAt<T>(this T[,,] array, int i, int j, int k);
        public static Span<T> AsSpan<T>(this T[,,]? array);
        public static Memory<T> AsMemory<T>(this T[,,]? array);
        public static Span<T> AsSpan<T>(this T[,,] array, int depth);
        public static Memory<T> AsMemory<T>(this T[,,] array, int depth);
        public static Span2D<T> AsSpan2D<T>(this T[,,] array, int depth);
        public static Memory2D<T> AsMemory2D<T>(this T[,,] array, int depth);
        public static int Count<T>(this T[,,] array, T value) where T : IEquatable<T>;
        public static int GetDjb2HashCode<T>(this T[,,] array);
        public static bool IsCovariant<T>(this T[,,] array);
    }

    public static class MemoryExtensions
    {
        public static Memory2D<T> AsMemory2D<T>(this Memory<T> memory, int height, int width);
        public static Memory2D<T> AsMemory2D<T>(this Memory<T> memory, int offset, int height, int width, int pitch);
    }

    public static class ReadOnlyMemoryExtensions
    {
        public static ReadOnlyMemory2D<T> AsMemory2D<T>(this ReadOnlyMemory<T> memory, int height, int width);
        public static ReadOnlyMemory2D<T> AsMemory2D<T>(this ReadOnlyMemory<T> memory, int offset, int height, int width, int pitch);
    }

    public static class ReadOnlySpanExtensions
    {
        public static ReadOnlySpan2D<T> AsMemory2D<T>(this ReadOnlySpan<T> span, int height, int width);
        public static ReadOnlySpan2D<T> AsMemory2D<T>(this ReadOnlySpan<T> span, int offset, int height, int width, int pitch);
        public static void CopyTo<T>(this ReadOnlySpan<T> span, RefEnumerable<T> destination);
        public static bool TryCopyTo<T>(this ReadOnlySpan<T> span, RefEnumerable<T> destination);
    }

    public static class SpanExtensions
    {
        public static Span2D<T> AsMemory2D<T>(this Span<T> span, int height, int width);
        public static Span2D<T> AsMemory2D<T>(this Span<T> span, int offset, int height, int width, int pitch);
        public static void CopyTo<T>(this Span<T> span, RefEnumerable<T> destination);
        public static bool TryCopyTo<T>(this Span<T> span, RefEnumerable<T> destination);
    }
}

namespace Microsoft.Toolkit.HighPerformance.Helpers
{
    public static partial class ParallelHelper
    {
        public static void ForEach<TItem, TAction>(Memory2D<TItem> memory) where TAction : struct, IRefAction<TItem>;
        public static void ForEach<TItem, TAction>(Memory2D<TItem> memory, int minimumActionsPerThread) where TAction : struct, IRefAction<TItem>;
        public static void ForEach<TItem, TAction>(Memory2D<TItem> memory, in TAction action) where TAction : struct, IRefAction<TItem>;
        public static void ForEach<TItem, TAction>(Memory2D<TItem> memory, in TAction action, int minimumActionsPerThread) where TAction : struct, IRefAction<TItem>;
        public static void ForEach<TItem, TAction>(ReadOnlyMemory2D<TItem> memory) where TAction : struct, IInAction<TItem>;
        public static void ForEach<TItem, TAction>(ReadOnlyMemory2D<TItem> memory, int minimumActionsPerThread) where TAction : struct, IInAction<TItem>;
        public static void ForEach<TItem, TAction>(ReadOnlyMemory2D<TItem> memory, in TAction action) where TAction : struct, IInAction<TItem>;
        public static void ForEach<TItem, TAction>(ReadOnlyMemory2D<TItem> memory, in TAction action, int minimumActionsPerThread) where TAction : struct, IInAction<TItem>;
    }
}

namespace Microsoft.Toolkit.HighPerformance.Memory
{
    public readonly struct Memory2D<T> : IEquatable<Memory2D<T>>
    {
        public Memory2D(T[] array, int height, int width);
        public Memory2D(T[] array, int offset, int height, int width, int pitch);
        public Memory2D(T[,]? array);
        public Memory2D(T[,]? array, int row, int column, int height, int width);
        public Memory2D(T[,,] array, int depth);
        public Memory2D(T[,,] array, int depth, int row, int column, int height, int width);
        public Memory2D(MemoryManager<T> memoryManager, int height, int width);
        public Memory2D(MemoryManager<T> memoryManager, int offset, int height, int width, int pitch);

        public static Memory2D<T> DangerousCreate(object instance, ref T value, int height, int width, int pitch);

        public static Memory2D<T> Empty { get; }

        public bool IsEmpty { get; }
        public nint Length { get; }
        public int Height { get; }
        public int Width { get; }
        public Span2D<T> Span { get; }
        public Memory2D<T> this[Range rows, Range columns] { get; }

        public Memory2D<T> Slice(int row, int column, int height, int width);
        public void CopyTo(Memory<T> destination);
        public bool TryCopyTo(Memory<T> destination);
        public void CopyTo(Memory2D<T> destination);
        public bool TryCopyTo(Memory2D<T> destination);
        public MemoryHandle Pin();
        public bool TryGetMemory(out Memory<T> memory);
        public T[,] ToArray();
        public override bool Equals(object? obj);
        public bool Equals(Memory2D<T> other);
        public override int GetHashCode();
        public override string ToString();

        public static implicit operator Memory2D<T>(T[,]? array) => new Memory2D<T>(array);
    }

    public readonly struct ReadOnlyMemory2D<T> : IEquatable<ReadOnlyMemory2D<T>>
    {
        public ReadOnlyMemory2D(string text, int height, int width);
        public ReadOnlyMemory2D(string text, int offset, int height, int width, int pitch);
        public ReadOnlyMemory2D(T[] array, int height, int width);
        public ReadOnlyMemory2D(T[] array, int offset, int height, int width, int pitch);
        public ReadOnlyMemory2D(T[,]? array);
        public ReadOnlyMemory2D(T[,]? array, int row, int column, int height, int width);
        public ReadOnlyMemory2D(T[,,] array, int depth);
        public ReadOnlyMemory2D(T[,,] array, int depth, int row, int column, int height, int width);
        public ReadOnlyMemory2D(MemoryManager<T> memoryManager, int height, int width);
        public ReadOnlyMemory2D(MemoryManager<T> memoryManager, int offset, int height, int width, int pitch);

        public static ReadOnlyMemory2D<T> DangerousCreate(object instance, ref T value, int height, int width, int pitch);

        public static ReadOnlyMemory2D<T> Empty  { get; }
        public bool IsEmpty { get; }
        public nint Length { get; }
        public int Height { get; }
        public int Width { get; }
        public ReadOnlySpan2D<T> Span { get; }
        public ReadOnlyMemory2D<T> this[Range rows, Range columns] { get; }

        public ReadOnlyMemory2D<T> Slice(int row, int column, int height, int width);
        public void CopyTo(Memory<T> destination);
        public bool TryCopyTo(Memory<T> destination);
        public void CopyTo(Memory2D<T> destination);
        public bool TryCopyTo(Memory2D<T> destination);
        public MemoryHandle Pin();
        public bool TryGetMemory(out ReadOnlyMemory<T> memory);
        public T[,] ToArray();
        public override bool Equals(object? obj);
        public bool Equals(ReadOnlyMemory2D<T> other);
        public override int GetHashCode();
        public override string ToString();

        public static implicit operator ReadOnlyMemory2D<T>(T[,]? array);
        public static implicit operator ReadOnlyMemory2D<T>(Memory2D<T> memory);
    }

    public readonly ref struct Span2D<T>
    {
        public Span2D(void* pointer, int height, int width, int pitch);
        public Span2D(T[] array, int height, int width);
        public Span2D(T[] array, int offset, int height, int width, int pitch);
        public Span2D(T[,]? array);
        public Span2D(T[,]? array, int row, int column, int height, int width);
        public Span2D(T[,,] array, int depth);
        public Span2D(T[,,] array, int depth, int row, int column, int height, int width);

        public static Span2D<T> DangerousCreate(ref T value, int height, int width, int pitch);

        public static Span2D<T> Empty { get; }
        public bool IsEmpty { get; }
        public nint Length { get; }
        public int Height { get; }
        public int Width { get; }
        public ref T this[int row, int column] { get; }
        public ref T this[Index row, Index column] { get; }
        public Span2D<T> this[Range rows, Range columns] { get; }

        public void Clear();
        public void CopyTo(Span<T> destination);
        public void CopyTo(Span2D<T> destination);
        public bool TryCopyTo(Span<T> destination);
        public bool TryCopyTo(Span2D<T> destination);
        public void Fill(T value);
        public ref T GetPinnableReference();
        public ref T DangerousGetReference();
        public ref T DangerousGetReferenceAt(int i, int j);
        public Span2D<T> Slice(int row, int column, int height, int width);
        public Span<T> GetRowSpan(int row);
        public bool TryGetSpan(out Span<T> span);
        public T[,] ToArray();
        public override bool Equals(object? obj);
        public override int GetHashCode();
        public override string ToString();

        public static bool operator ==(Span2D<T> left, Span2D<T> right);
        public static bool operator !=(Span2D<T> left, Span2D<T> right);
        public static implicit operator Span2D<T>(T[,]? array);

        public RefEnumerable<T> GetRow(int row);
        public RefEnumerable<T> GetColumn(int column);
        public Enumerator GetEnumerator();

        public ref struct Enumerator
        {
            public bool MoveNext();
            public readonly ref T Current { get; }
        }
    }

    public readonly ref struct ReadOnlySpan2D<T>
    {
        public ReadOnlySpan2D(void* pointer, int height, int width, int pitch);
        public ReadOnlySpan2D(T[] array, int height, int width);
        public ReadOnlySpan2D(T[] array, int offset, int height, int width, int pitch);
        public ReadOnlySpan2D(T[,]? array);
        public ReadOnlySpan2D(T[,]? array, int row, int column, int height, int width);
        public ReadOnlySpan2D(T[,,] array, int depth);
        public ReadOnlySpan2D(T[,,] array, int depth, int row, int column, int height, int width);

        public static ReadOnlySpan2D<T> DangerousCreate(in T value, int height, int width, int pitch);

        public static ReadOnlySpan2D<T> Empty { get; }
        public bool IsEmpty { get; }
        public nint Length { get; }
        public int Height { get; }
        public int Width { get; }
        public ref readonly T this[int row, int column] { get; }
        public ref readonly T this[Index row, Index column] { get; }
        public ReadOnlySpan2D<T> this[Range rows, Range columns] { get; }

        public void CopyTo(Span<T> destination);
        public void CopyTo(Span2D<T> destination);
        public bool TryCopyTo(Span<T> destination);
        public bool TryCopyTo(Span2D<T> destination);
        public ref T GetPinnableReference();
        public ref T DangerousGetReference();
        public ref T DangerousGetReferenceAt(int i, int j);
        public ReadOnlySpan2D<T> Slice(int row, int column, int width, int height);
        public ReadOnlySpan<T> GetRowSpan(int row);
        public bool TryGetSpan(out ReadOnlySpan<T> span);
        public T[,] ToArray();
        public override bool Equals(object? obj);
        public override int GetHashCode();
        public override string ToString();

        public static bool operator ==(ReadOnlySpan2D<T> left, ReadOnlySpan2D<T> right);
        public static bool operator !=(ReadOnlySpan2D<T> left, ReadOnlySpan2D<T> right);
        public static implicit operator ReadOnlySpan2D<T>(T[,]? array);
        public static implicit operator ReadOnlySpan2D<T>(Span2D<T> span);

        public ReadOnlyRefEnumerable<T> GetRow(int row);
        public ReadOnlyRefEnumerable<T> GetColumn(int column);
        public Enumerator GetEnumerator();

        public ref struct Enumerator
        {
            public bool MoveNext();
            public readonly ref readonly T Current { get; }
        }
    }
}

These two types are also mirrored as ReadOnlyMemory2D<T> and ReadOnlySpan2D<T>.

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Pull Request has been submitted to the documentation repository instructions. Link: #376
~~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~~
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

Additional notes

The breaking change is the removal of the Array2DRowEnumerable<T> and Array2DColumnEnumerable<T> types, which were used in the GetRow and GetColumn APIs. This approach had two problems:

There were two completely different iterator types, one per API
The GetRow API was a breaking change when upgrading, as the return type was just Span<T> there.

The solution, as discussed with @john-h-k and @tannergooding, was to introduce a new, shared RefEnumerable<T> type that can be used to traverse both rows and columns, and to use that one for both APIs. Next to them, a new API GetRowSpan was also added only on supported runtimes (so not a breaking change), which is equivalent to GetRow but returning a Span<T> value.

msftbot · 2020-06-17T14:57:49Z

Thanks Sergio0694 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

Microsoft.Toolkit.HighPerformance/Memory/Memory2D{T}.cs

Microsoft.Toolkit.HighPerformance/Memory/Span2D{T}.cs

Microsoft.Toolkit.HighPerformance/Enumerables/ReadOnlyRefEnumerable{T}.cs

Microsoft.Toolkit.HighPerformance/Extensions/ArrayExtensions.1D.cs

...ft.Toolkit.HighPerformance/Helpers/ParallelHelper.ForEach.IInAction2D.cs

...t.Toolkit.HighPerformance/Helpers/ParallelHelper.ForEach.IRefAction2D.cs

msftbot · 2020-06-19T21:41:36Z

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

This reverts commit 78a0266. Done to avoid conflicts, since these two enumerable types are already being overhauled in windows-toolkit#3353 anyway.

michael-hawker

Going to wait until #3351 gets merged.

Microsoft.Toolkit.HighPerformance/Enumerables/ReadOnlySpanEnumerable{T}.cs

HurricanKai

I looked at most of this code, and while I'm not 100% sure, I think it all looks good to me.
I've got 2 usability issues though:

why are the width/height indexes called i, j? why not x, y?
Why do Equals & GetHashCode just throw?

Microsoft.Toolkit.HighPerformance/Memory/Span2D{T}.cs

Sergio0694 · 2020-09-10T15:47:39Z

Hey @HurricanKai, thanks for the review! 😊

As for your questions:

I called them i and j mostly out of habit, I suppose it would make sense to call them y and x, yes (not x and y though, the first order is the vertical coordinate)
Equals and GetHashCode throw to mirror the behavior of the Span<T> types in the BCL, which do the same (you can see the code in CoreCLR here). Like with those types, we're throwing here and instead implementing the == and != operators. I believe the intent here is that == is just used to indicate whether two memory regions map to the same area, while using GetHashCode and Equals could be confusing since they would not actually check the contents, but just the internal reference. So two different areas with the same contents would not be reported as being the same. Granted, that's the same for eg. arrays, but... I'm really just following their example here to make things consistent with the BCL 😄

michael-hawker · 2020-09-25T20:49:17Z

@Sergio0694 this has some conflicts to resolve now?

Sergio0694 · 2020-09-25T21:20:30Z

@michael-hawker Yup, was just waiting on #3378 to be merged first since that'll edit some of the same files anyway, so that here I'll just have to resolve conflicts once when pulling changes from master from both the two other PRs for this package 😄

michael-hawker · 2020-10-08T20:18:06Z

@Sergio0694 #3378 is merged now, can you resolve conflicts and clean-up this PR so we can re-review and get it merged as well? Thanks! 🙂

michael-hawker

Few small comments on the unit tests, looking good!

michael-hawker · 2020-11-03T21:42:27Z

Microsoft.Toolkit.HighPerformance/Extensions/HashCodeExtensions.cs

@@ -23,12 +23,7 @@ public static class HashCodeExtensions
        /// <param name="span">The input <see cref="ReadOnlySpan{T}"/> instance.</param>
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static void Add<T>(ref this HashCode hashCode, ReadOnlySpan<T> span)
-#if SPAN_RUNTIME_SUPPORT


This has changed now I guess from the runtime side?

I've removed that restriction because I've now added polyfills for the RuntimeHelpers.IsReferenceOrContainsReferences<T>() API on .NET Standard 2.0 and below (I needed this for other code in the Memory2D<T> and Span2D<T> types, so I used it here too while I was at it), I use that here:

WindowsCommunityToolkit/Microsoft.Toolkit.HighPerformance/Helpers/HashCode{T}.cs

Lines 62 to 65 in 2745962

// If typeof(T) is not unmanaged, iterate over all the items one by one.

// This check is always known in advance either by the JITter or by the AOT

// compiler, so this branch will never actually be executed by the code.

if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())

Now that older targets can test this too at runtime, we no longer have to add that extra type constraint, which was basically just a precaution due to the lack of this API at runtime. You can see how we switch the polyfill on here:

WindowsCommunityToolkit/Microsoft.Toolkit.HighPerformance/Helpers/HashCode{T}.cs

Lines 12 to 16 in 2745962

#if SPAN_RUNTIME_SUPPORT

using RuntimeHelpers = System.Runtime.CompilerServices.RuntimeHelpers;

#else

using RuntimeHelpers = Microsoft.Toolkit.HighPerformance.Helpers.Internals.RuntimeHelpers;

#endif

So now on all targets the API will just automatically pick the right implementation for each input type T 😄

michael-hawker · 2020-11-03T21:42:27Z

UnitTests/UnitTests.HighPerformance.Shared/Extensions/Test_ArrayExtensions.2D.cs

+
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => array.GetColumn(-1));
+
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => array.GetColumn(20));


We should also be testing the 'off-by-1' index of '3', eh? (here and where-ever)

Right! I had added those tests when getting rows/columns from spans, but not from 2D arrays directly!
Fixed in 7a65a29, for both GetRow and GetColumn in this file 😄

michael-hawker · 2020-11-03T21:42:27Z

UnitTests/UnitTests.HighPerformance.Shared/Extensions/Test_ReadOnlySpanExtensions.cs

+
+            // Try to copy to a valid row and an invalid column (too short for the source span)
+            bool shouldBeTrue = values.TryCopyTo(array1.GetRow(2));
+            bool shouldBeFalse = values.TryCopyTo(array1.GetColumn(1));


We're not checking here that no mutation occurred, eh? As the line 281 above would have copied the same values? (I mean we know it failed as '20' is still in the 3rd row, but may be better to use a different column like '3' here to be safe to guard against this case too.)

Good catch! Also added one extra check for the entire target array for good measure 😄
Done in 6bb9e5a, including the same changes for the comment below.

...Tests/UnitTests.HighPerformance.Shared/Extensions/Test_SpanExtensions.cs

michael-hawker · 2020-11-03T21:42:27Z

Microsoft.Toolkit.HighPerformance/Helpers/ParallelHelper.ForEach.IRefAction.cs

@@ -135,8 +135,7 @@ public static partial class ParallelHelper
            /// Processes the batch of actions at a specified index
            /// </summary>
            /// <param name="i">The index of the batch to process</param>
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]


Expected for the inlining to disappear?

Yeah this was just a leftover, it's not there in the other implementations either. This method is just called once per thread anyway and does all the job. It's the actual delegates within each invocation of this method that need to be inlined, as they're invoked in a tight loop, not this one. As in, this loop here:

WindowsCommunityToolkit/Microsoft.Toolkit.HighPerformance/Helpers/ParallelHelper.ForEach.IRefAction.cs

Lines 147 to 152 in 2745962

for (int j = low; j < end; j++)

{

ref TItem rj = ref Unsafe.Add(ref r0, (nint)(uint)j);

Unsafe.AsRef(this.action).Invoke(ref rj);

}

michael-hawker · 2020-11-03T21:42:27Z

Microsoft.Toolkit.HighPerformance/Helpers/ParallelHelper.ForEach.IRefAction2D.cs

+            // of CPU cores (which is an int), and the height of each batch is necessarily
+            // smaller than or equal than int.MaxValue, as it can't be greater than the
+            // number of total batches, which again is capped at the number of CPU cores.
+            nint


So if this is native to the platform, how do we unit test the code for both possible sizes?

michael-hawker · 2020-11-03T21:42:27Z

Microsoft.Toolkit.HighPerformance/Microsoft.Toolkit.HighPerformance.csproj

@@ -2,7 +2,7 @@

  <PropertyGroup>
    <TargetFrameworks>netstandard1.4;netstandard2.0;netstandard2.1;netcoreapp2.1;netcoreapp3.1</TargetFrameworks>
-    <LangVersion>8.0</LangVersion>
+    <LangVersion>9.0</LangVersion>


Does this need anything new for our build pipeline/tooling requirements for folks?

Commenting here about the same comments from the stream, for future reference. Right now this works on the latest versions of VS stable (not the preview preview), because it uses the same Roslyn codebase as VS Preview, just from an older branch. As such, some C# 9 features are buggy/outdated (eg. function pointers), but since here we're really only using the new nint type, that bit in particular mostly works. I only had to add an extra explicit cast here to ensure the correct codegen due to a small compiler bug that's fixed in VS Preview, for instance:

WindowsCommunityToolkit/Microsoft.Toolkit.HighPerformance/Helpers/Internals/SpanHelper.Count.cs

Lines 277 to 280 in 7a65a29

// The (void*) cast is necessary to ensure the right constant is produced on runtimes

// before .NET 5 that don't natively support C# 9. For instance, removing that (void*)

// cast results in the value 0xFFFFFFFFFFFFFFFF (-1) instead of 0x7FFFFFFFFFFFFFFFF.

return (nint)(void*)long.MaxValue;

As in, this PR should work fine when compiled with VS stable, but especially due to all the other C# 9 changes coming in other PRs, as well as the .NET 5 support itself, we should definitely update the CI build of VS to the new release as soon as it drops hopefully next week when .NET 5 is announced at .NET Conf 😄

michael-hawker · 2020-11-03T21:42:27Z

UnitTests/UnitTests.HighPerformance.Shared/Extensions/Test_ReadOnlySpanExtensions.cs

+
+            // Copy a span to a target row and column with valid lengths
+            values.CopyTo(array1.GetRow(0));
+            values.Slice(0, 4).CopyTo(array1.GetColumn(1));


Should we check the intermediary results? I've noticed these tests are chaining quite a few operations together, so it may be better to help find problems in a particular part if we're validating the first part before moving on? (Or split them as separate tests?)

I think I addressed this in 6bb9e5a, let me know if that's alright 😄

michael-hawker · 2020-11-03T21:42:27Z

UnitTests/UnitTests.HighPerformance.Shared/Memory/Test_Memory2D{T}.cs

+
+            // A couple of tests for invalid parameters, ie. layers out of range
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => new Memory2D<int>(array, -1));
+            Assert.ThrowsException<ArgumentOutOfRangeException>(() => new Memory2D<int>(array, 20));


same here for off-by-one

Added a few new tests for this and ReadOnlyMemory2D<T> versions in 7b04c7d 👍

msftbot · 2020-11-03T21:45:41Z

Hello @michael-hawker!

Because this pull request has the auto merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

Microsoft.Toolkit.HighPerformance/Memory/Span2D{T}.cs

RosarioPulella

Other than cleaning up some no longer needed SuppressMessageAttributes, It looks good.

RosarioPulella · 2020-11-05T00:16:24Z

UnitTests/UnitTests.HighPerformance.Shared/Extensions/Test_ArrayExtensions.2D.cs

        [SuppressMessage("StyleCop.CSharp.NamingRules", "SA1312", Justification = "Dummy loop variable")]
        [SuppressMessage("StyleCop.CSharp.LayoutRules", "SA1501", Justification = "Empty test loop")]


There a few SuppressMessageAttributes left on methods that are no longer needed. It would be good to remove them from methods that no longer need it.

Oh, good catch, forgot about those! 😄
Fixed in 7e67a9c, and also fixed some other leftover warnings in the tests.

Sergio0694 · 2020-11-05T00:22:55Z

Removing the auto merge tag now that the PR is approved as I was waiting on one last piece of feedback from Tanner on a possible minor tweak to do to some constructors in the new types, so that if that's needed I could just do it here instead of opening a whole other PR just to fix that afterwards? cc. @michael-hawker

msftbot bot assigned michael-hawker Jun 17, 2020

john-h-k reviewed Jun 17, 2020

View changes

Sergio0694 marked this pull request as ready for review Jun 19, 2020

john-h-k suggested changes Jun 19, 2020

View changes

msftbot bot added needs author feedback 📝 needs attention 👋 and removed needs author feedback 📝 labels Jun 19, 2020

Sergio0694 added feature 💡 high-performance 🚂 labels Jul 7, 2020

This was referenced Aug 16, 2020

Restructure sections to separate UWP and .NET Standard MicrosoftDocs/WindowsCommunityToolkitDocs#303

Open

[Future] 7.0 Release Plan #3062

Open

Memory2D<T>/Span2D<T> docs MicrosoftDocs/WindowsCommunityToolkitDocs#376

Open

michael-hawker reviewed Sep 8, 2020

View changes

Microsoft.Toolkit.HighPerformance/Enumerables/ReadOnlySpanEnumerable{T}.cs Outdated Show resolved Hide resolved

HurricanKai reviewed Sep 10, 2020

View changes

Microsoft.Toolkit.HighPerformance/Memory/Span2D{T}.cs Outdated Show resolved Hide resolved

Sergio0694 mentioned this pull request Sep 24, 2020

Deprecated 2D array extensions from Microsoft.Toolkit #3444

Merged

4 of 7 tasks complete

msftbot bot removed the needs attention 👋 label Sep 25, 2020

Sergio0694 added 7 commits Jun 17, 2020

Added new constructors for Ref<T> and ReadOnlyRef<T>

2b80061

Added initial version of the Span2D<T> type

b89a83c

Minor code tweaks to Span2D<T>

d6980bc

Added initial version of the Memory2D<T> type

eb1bb6e

Added missing Memory2D<T> header

f11b77d

Improved Span2D<T> layout (removed 4 bytes)

64a293c

Minor code refactoring

4de84df

Sergio0694 and others added 9 commits Oct 31, 2020

Tweaked some using directives

296e916

Improved unit test coverage for RefEnumerable<T>

Loading status checks…

14f40e2

Removed unnecessary covariance array checks

2626c7d

Added comments to unit tests

Loading status checks…

8a70ace

Fixed bugs in unit tests

Loading status checks…

151f38a

Fixed bug in ReadOnlyRefEnumerable<T> implicit cast

Loading status checks…

20bd65f

Minor code refactoring

Loading status checks…

f3e7e00

Minor code tweaks for clarity

Loading status checks…

bc6f70b

Merge branch 'master' into feature/array3d-apis

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

2745962

michael-hawker approved these changes Nov 3, 2020

View changes

michael-hawker added this to the 7.0 milestone Nov 3, 2020

michael-hawker added this to In progress in Features 7.0 via automation Nov 3, 2020

michael-hawker added the auto merge ⚡ label Nov 3, 2020

DaZombieKiller reviewed Nov 3, 2020

View changes

Microsoft.Toolkit.HighPerformance/Memory/Span2D{T}.cs Outdated Show resolved Hide resolved

Sergio0694 added 6 commits Nov 3, 2020

Added tests for off-by-one Get[Row|Column] for 2D arrays

Loading status checks…

7a65a29

Improved some unit tests

Loading status checks…

6bb9e5a

Added more tests for invalid Memory2D<T> constructors

Loading status checks…

7b04c7d

Fixed some typos

Loading status checks…

7738d34

Fixed XML docs for internal helpers

Loading status checks…

cd21086

Updated package description with new types

Loading status checks…

9c90f66

RosarioPulella approved these changes Nov 5, 2020

View changes

Features 7.0 automation moved this from In progress to Reviewer approved Nov 5, 2020

Sergio0694 removed the auto merge ⚡ label Nov 5, 2020

Fixed StyleCop warnings in HighPerformance tests

Loading status checks…

7e67a9c

michael-hawker added the auto merge ⚡ label Nov 5, 2020

Features 7.0 automation moved this from Reviewer approved to Done Nov 5, 2020

windows-toolkit / WindowsCommunityToolkit

2D System.Memory-like primitives (Span2D<T>, Memory2D<T>) #3353

2D System.Memory-like primitives (Span2D<T>, Memory2D<T>) #3353

Sergio0694 commented Jun 17, 2020 •

edited

msftbot bot commented Jun 17, 2020

msftbot bot commented Jun 19, 2020

michael-hawker left a comment

HurricanKai left a comment

Sergio0694 commented Sep 10, 2020

michael-hawker commented Sep 25, 2020

Sergio0694 commented Sep 25, 2020

michael-hawker commented Oct 8, 2020

michael-hawker left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

msftbot bot commented Nov 3, 2020

RosarioPulella left a comment

This comment has been minimized.

This comment has been minimized.

Sergio0694 commented Nov 5, 2020

	// If typeof(T) is not unmanaged, iterate over all the items one by one.
	// This check is always known in advance either by the JITter or by the AOT
	// compiler, so this branch will never actually be executed by the code.
	if (RuntimeHelpers.IsReferenceOrContainsReferences<T>())

	#if SPAN_RUNTIME_SUPPORT
	using RuntimeHelpers = System.Runtime.CompilerServices.RuntimeHelpers;
	#else
	using RuntimeHelpers = Microsoft.Toolkit.HighPerformance.Helpers.Internals.RuntimeHelpers;
	#endif


		Assert.ThrowsException<ArgumentOutOfRangeException>(() => array.GetColumn(-1));

		Assert.ThrowsException<ArgumentOutOfRangeException>(() => array.GetColumn(20));

	for (int j = low; j < end; j++)
	{
	ref TItem rj = ref Unsafe.Add(ref r0, (nint)(uint)j);

	Unsafe.AsRef(this.action).Invoke(ref rj);
	}

	// The (void*) cast is necessary to ensure the right constant is produced on runtimes
	// before .NET 5 that don't natively support C# 9. For instance, removing that (void*)
	// cast results in the value 0xFFFFFFFFFFFFFFFF (-1) instead of 0x7FFFFFFFFFFFFFFFF.
	return (nint)(void*)long.MaxValue;

		[SuppressMessage("StyleCop.CSharp.NamingRules", "SA1312", Justification = "Dummy loop variable")]
		[SuppressMessage("StyleCop.CSharp.LayoutRules", "SA1501", Justification = "Empty test loop")]

windows-toolkit / WindowsCommunityToolkit

2D System.Memory-like primitives (Span2D<T>, Memory2D<T>) #3353

2D System.Memory-like primitives (Span2D<T>, Memory2D<T>) #3353

Conversation

Sergio0694 commented Jun 17, 2020 • edited

PR Type

What is the current behavior?

What is the new behavior?

API surface (summary)

PR Checklist

Additional notes

msftbot bot commented Jun 17, 2020

msftbot bot commented Jun 19, 2020

michael-hawker left a comment

HurricanKai left a comment

Sergio0694 commented Sep 10, 2020

michael-hawker commented Sep 25, 2020

Sergio0694 commented Sep 25, 2020

michael-hawker commented Oct 8, 2020

michael-hawker left a comment

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

Sergio0694 Nov 3, 2020 Author Member

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

Sergio0694 Nov 3, 2020 Author Member

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

Sergio0694 Nov 3, 2020 Author Member

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

Sergio0694 Nov 3, 2020 Author Member

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

Sergio0694 Nov 3, 2020 Author Member

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

Sergio0694 Nov 3, 2020 Author Member

This comment has been minimized.

michael-hawker Nov 3, 2020 Member

This comment has been minimized.

Sergio0694 Nov 3, 2020 Author Member

msftbot bot commented Nov 3, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

RosarioPulella left a comment

This comment has been minimized.

RosarioPulella Nov 5, 2020 Contributor

This comment has been minimized.

Sergio0694 Nov 5, 2020 Author Member

Sergio0694 commented Nov 5, 2020

Sergio0694 commented Jun 17, 2020 •

edited

michael-hawker Nov 3, 2020
Member

Sergio0694 Nov 3, 2020
Author Member

michael-hawker Nov 3, 2020
Member

Sergio0694 Nov 3, 2020
Author Member

michael-hawker Nov 3, 2020
Member

Sergio0694 Nov 3, 2020
Author Member

michael-hawker Nov 3, 2020
Member

Sergio0694 Nov 3, 2020
Author Member

michael-hawker Nov 3, 2020
Member

michael-hawker Nov 3, 2020
Member

Sergio0694 Nov 3, 2020
Author Member

michael-hawker Nov 3, 2020
Member

Sergio0694 Nov 3, 2020
Author Member

michael-hawker Nov 3, 2020
Member

Sergio0694 Nov 3, 2020
Author Member

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

RosarioPulella Nov 5, 2020
Contributor

Sergio0694 Nov 5, 2020
Author Member