From 1ea0328eb7e8e42db9af948e653ae143ac1bf07f Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Fri, 1 May 2020 16:08:53 -0700 Subject: [PATCH 1/2] Prefer Buffer.Memmove over Array.Copy in some scenarios --- .../src/System/Array.cs | 13 ++++++-- .../CompilerServices/RuntimeHelpers.cs | 30 ++++++++++++------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.cs b/src/libraries/System.Private.CoreLib/src/System/Array.cs index fa80d26e412b21..2c67462e50453f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.cs @@ -51,7 +51,7 @@ public static void Resize([NotNull] ref T[]? array, int newSize) if (newSize < 0) ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.newSize, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); - T[]? larray = array; + T[]? larray = array; // local copy if (larray == null) { array = new T[newSize]; @@ -60,8 +60,17 @@ public static void Resize([NotNull] ref T[]? array, int newSize) if (larray.Length != newSize) { + // Due to array variance, it's possible that the incoming array is + // actually of type U[], where U:T; or that an int[] <-> uint[] or + // similar cast has occurred. In any case, since it's always legal + // to reinterpret U as T in this scenario (but not necessarily the + // other way around), we can use Buffer.Memmove here. + T[] newArray = new T[newSize]; - Copy(larray, 0, newArray, 0, larray.Length > newSize ? newSize : larray.Length); + Buffer.Memmove( + ref MemoryMarshal.GetArrayDataReference(newArray), + ref MemoryMarshal.GetArrayDataReference(larray), + (uint)Math.Min(newSize, larray.Length)); array = newArray; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs index 3f06b3def7f29c..0790cdaab4663e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs @@ -27,6 +27,8 @@ public static T[] GetSubArray(T[] array, Range range) (int offset, int length) = range.GetOffsetAndLength(array.Length); + T[] dest; + if (typeof(T).IsValueType || typeof(T[]) == array.GetType()) { // We know the type of the array to be exactly T[]. @@ -36,20 +38,28 @@ public static T[] GetSubArray(T[] array, Range range) return Array.Empty(); } - var dest = new T[length]; - Buffer.Memmove( - ref MemoryMarshal.GetArrayDataReference(dest), - ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), offset), - (uint)length); - return dest; + dest = new T[length]; } else { - // The array is actually a U[] where U:T. - T[] dest = (T[])Array.CreateInstance(array.GetType().GetElementType()!, length); - Array.Copy(array, offset, dest, 0, length); - return dest; + // The array is actually a U[] where U:T. We'll make sure to create + // an array of the exact same backing type. The cast to T[] will + // never fail. + + dest = Unsafe.As(Array.CreateInstance(array.GetType().GetElementType()!, length)); } + + // In either case, the newly-allocated array is the exact same type as the + // original incoming array. It's safe for us to Buffer.Memmove the contents + // from the source array to the destination array, otherwise the contents + // wouldn't have been valid for the source array in the first place. + + Buffer.Memmove( + ref MemoryMarshal.GetArrayDataReference(dest), + ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), offset), + (uint)length); + + return dest; } public static object GetUninitializedObject(Type type) From f3fa24548075a8e530943bdb168efd45c7714ff3 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 6 May 2020 16:29:31 -0700 Subject: [PATCH 2/2] PR feedback --- src/libraries/System.Private.CoreLib/src/System/Array.cs | 2 +- .../src/System/Runtime/CompilerServices/RuntimeHelpers.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.cs b/src/libraries/System.Private.CoreLib/src/System/Array.cs index 2c67462e50453f..fc015781544228 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.cs @@ -67,7 +67,7 @@ public static void Resize([NotNull] ref T[]? array, int newSize) // other way around), we can use Buffer.Memmove here. T[] newArray = new T[newSize]; - Buffer.Memmove( + Buffer.Memmove( ref MemoryMarshal.GetArrayDataReference(newArray), ref MemoryMarshal.GetArrayDataReference(larray), (uint)Math.Min(newSize, larray.Length)); diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs index 0790cdaab4663e..921f447bb6fcf4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.cs @@ -31,7 +31,8 @@ public static T[] GetSubArray(T[] array, Range range) if (typeof(T).IsValueType || typeof(T[]) == array.GetType()) { - // We know the type of the array to be exactly T[]. + // We know the type of the array to be exactly T[] or an array variance + // compatible value type substitution like int[] <-> uint[]. if (length == 0) {