Fix MemoryManager ctor, add unit and perf tests, and improve performa…

…nce (#28880) * Fix MemoryManager ctor, add unit and perf tests, and improve performance. * Remove Dangerous Span Ctor * Fix sort order in csproj and rename Perf.MemorySlice.cs to Perf.Memory.Slice * Fix MemoryManager ctor and use internal span ctor to improve performance (#17452) * Fix MemoryManager ctor, add unit and perf tests, and use internal span ctor. * Address PR feedback (remove use of Unsafe.As and Dangerous Span Ctor) Signed-off-by: dotnet-bot-corefx-mirror <[email protected]>
dotnet · Apr 10, 2018 · 1dfe097 · 1dfe097
1 parent 14f5d53
commit 1dfe097
Show file tree

Hide file tree

Showing 9 changed files with 264 additions and 9 deletions.
diff --git a/src/Common/src/CoreLib/System/Memory.cs b/src/Common/src/CoreLib/System/Memory.cs
@@ -120,20 +120,17 @@ public Memory(T[] array, int start, int length)
         /// <param name="manager">The memory manager.</param>
         /// <param name="start">The index at which to begin the memory.</param>
         /// <param name="length">The number of items in the memory.</param>
-        /// <remarks>Returns default when <paramref name="manager"/> is null.</remarks>
+        /// <exception cref="System.ArgumentNullException">
+        /// Thrown when <paramref name="manager"/> is null.
+        /// </exception>
         /// <exception cref="System.ArgumentOutOfRangeException">
         /// Thrown when the specified <paramref name="start"/> or end index is not in the range (&lt;0 or &gt;=Length).
         /// </exception>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public Memory(MemoryManager<T> manager, int start, int length)
         {
             if (manager == null)
-            {
-                if (start != 0 || length != 0)
-                    ThrowHelper.ThrowArgumentOutOfRangeException();
-                this = default;
-                return; // returns default
-            }
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.manager);
             if ((uint)start > (uint)manager.Length || (uint)length > (uint)(manager.Length - start))
                 ThrowHelper.ThrowArgumentOutOfRangeException();
 
@@ -276,6 +273,7 @@ public Span<T> Span
                 if (_index < 0)
                 {
                     Debug.Assert(_length >= 0);
+                    Debug.Assert(_object != null);
                     return ((MemoryManager<T>)_object).GetSpan().Slice(_index & RemoveFlagsBitMask, _length);
                 }
                 else if (typeof(T) == typeof(char) && _object is string s)
@@ -335,6 +333,7 @@ public unsafe MemoryHandle Pin()
         {
             if (_index < 0)
             {
+                Debug.Assert(_object != null);
                 return ((MemoryManager<T>)_object).Pin((_index & RemoveFlagsBitMask));
             }
             else if (typeof(T) == typeof(char) && _object is string s)

diff --git a/src/Common/src/CoreLib/System/ReadOnlyMemory.cs b/src/Common/src/CoreLib/System/ReadOnlyMemory.cs
@@ -187,6 +187,7 @@ public ReadOnlySpan<T> Span
                 if (_index < 0)
                 {
                     Debug.Assert(_length >= 0);
+                    Debug.Assert(_object != null);
                     return ((MemoryManager<T>)_object).GetSpan().Slice(_index & RemoveFlagsBitMask, _length);
                 }
                 else if (typeof(T) == typeof(char) && _object is string s)
@@ -241,6 +242,7 @@ public unsafe MemoryHandle Pin()
         {
             if (_index < 0)
             {
+                Debug.Assert(_object != null);
                 return ((MemoryManager<T>)_object).Pin((_index & RemoveFlagsBitMask));
             }
             else if (typeof(T) == typeof(char) && _object is string s)

diff --git a/src/System.Memory/src/System/ThrowHelper.cs b/src/System.Memory/src/System/ThrowHelper.cs
@@ -185,6 +185,7 @@ internal enum ExceptionArgument
         startIndex,
         endIndex,
         array,
-        culture
+        culture,
+        manager
     }
 }
diff --git a/src/System.Memory/tests/Memory/MemoryManager.cs b/src/System.Memory/tests/Memory/MemoryManager.cs
@@ -27,6 +27,33 @@ public static void MemoryFromMemoryManagerInt()
             memory.Slice(4, 0).Validate();
         }
 
+        [Fact]
+        public static void MemoryManagerCtorDefault()
+        {
+            MemoryManager<int> managerInt = default;
+            Assert.Throws<ArgumentNullException>(() => new Memory<int>(managerInt, 0, 0));
+
+            managerInt = null;
+            Assert.Throws<ArgumentNullException>(() => new Memory<int>(managerInt, 0, 0));
+
+            MemoryManager<object> managerObject = default;
+            Assert.Throws<ArgumentNullException>(() => new Memory<object>(managerObject, 0, 0));
+        }
+
+        [Fact]
+        public static void MemoryManagerCtorInvalid()
+        {
+            int[] a = { 91, 92, -93, 94 };
+            MemoryManager<int> manager = new CustomMemoryForTest<int>(a);
+            Assert.Throws<ArgumentOutOfRangeException>(() => new Memory<int>(manager, 0, -1));
+            Assert.Throws<ArgumentOutOfRangeException>(() => new Memory<int>(manager, -1, 0));
+            Assert.Throws<ArgumentOutOfRangeException>(() => new Memory<int>(manager, -1, -1));
+            Assert.Throws<ArgumentOutOfRangeException>(() => new Memory<int>(manager, -1, -1));
+            Assert.Throws<ArgumentOutOfRangeException>(() => new Memory<int>(manager, 0, a.Length + 1));
+            Assert.Throws<ArgumentOutOfRangeException>(() => new Memory<int>(manager, a.Length + 1, 0));
+            Assert.Throws<ArgumentOutOfRangeException>(() => new Memory<int>(manager, 1, a.Length));
+        }
+
         [Fact]
         public static void ReadOnlyMemoryFromMemoryFromMemoryManagerInt()
         {

diff --git a/src/System.Memory/tests/Memory/Span.cs b/src/System.Memory/tests/Memory/Span.cs
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Buffers;
+using System.Runtime.InteropServices;
 using Xunit;
 
 namespace System.MemoryTests
@@ -41,6 +42,22 @@ public static void SpanFromCtorArrayLong()
             manager.Memory.Span.Validate(91, -92, 93, 94, -95);
         }
 
+        [Fact]
+        public static void SpanFromCtorArrayChar()
+        {
+            char[] a = { '1', '2', '3', '4', '-' };
+            Memory<char> memory;
+
+            memory = new Memory<char>(a);
+            memory.Span.Validate('1', '2', '3', '4', '-');
+
+            memory = new Memory<char>(a, 0, a.Length);
+            memory.Span.Validate('1', '2', '3', '4', '-');
+
+            MemoryManager<char> manager = new CustomMemoryForTest<char>(a);
+            manager.Memory.Span.Validate('1', '2', '3', '4', '-');
+        }
+
         [Fact]
         public static void SpanFromCtorArrayObject()
         {
@@ -59,6 +76,19 @@ public static void SpanFromCtorArrayObject()
             manager.Memory.Span.ValidateReferenceType(o1, o2);
         }
 
+        [Fact]
+        public static void SpanFromStringAsMemory()
+        {
+            string a = "1234-";
+            ReadOnlyMemory<char> memory;
+
+            memory = a.AsMemory();
+            MemoryMarshal.AsMemory(memory).Span.Validate('1', '2', '3', '4', '-');
+
+            memory = a.AsMemory(0, a.Length);
+            MemoryMarshal.AsMemory(memory).Span.Validate('1', '2', '3', '4', '-');
+        }
+
         [Fact]
         public static void SpanFromCtorArrayZeroLength()
         {

diff --git a/...ory/tests/Performance/Perf.MemorySlice.cs → ...ry/tests/Performance/Perf.Memory.Slice.cs b/...ory/tests/Performance/Perf.MemorySlice.cs → ...ry/tests/Performance/Perf.Memory.Slice.cs
diff --git a/src/System.Memory/tests/Performance/Perf.Memory.Span.cs b/src/System.Memory/tests/Performance/Perf.Memory.Span.cs
@@ -0,0 +1,166 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Buffers;
+using System.MemoryTests;
+using Microsoft.Xunit.Performance;
+using Xunit;
+
+namespace System.Memory.Tests
+{
+    public class Perf_Memory_Span
+    {
+        private const int InnerCount = 1_000_000;
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromDefaultIntegerMemory()
+        {
+            Memory<int> memory = default;
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        Span<int> span = memory.Span;
+                    }
+                }
+            }
+        }
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromIntegerArrayBackedMemory()
+        {
+            int[] a = { 91, 92, -93, 94 };
+            var memory = new Memory<int>(a);
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        Span<int> span = memory.Span;
+                    }
+                }
+            }
+        }
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromCharArrayBackedMemory()
+        {
+            char[] a = "9192-9394".ToCharArray();
+            var memory = new Memory<char>(a);
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        Span<char> span = memory.Span;
+                    }
+                }
+            }
+        }
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromObjectArrayBackedMemory()
+        {
+            object o1 = new object();
+            object o2 = new object();
+            object[] a = { o1, o2 };
+            var memory = new Memory<object>(a);
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        Span<object> span = memory.Span;
+                    }
+                }
+            }
+        }
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromStringBackedMemory()
+        {
+            string a = "9192-9394";
+            ReadOnlyMemory<char> memory = a.AsMemory();
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        ReadOnlySpan<char> span = memory.Span;
+                    }
+                }
+            }
+        }
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromIntegerMemoryManager()
+        {
+            int[] a = { 91, 92, -93, 94 };
+            var memory = new Memory<int>(a);
+            MemoryManager<int> manager = new CustomMemoryForTest<int>(a);
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        Span<int> span = manager.Memory.Span;
+                    }
+                }
+            }
+        }
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromCharMemoryManager()
+        {
+            char[] a = "9192-9394".ToCharArray();
+            var memory = new Memory<char>(a);
+            MemoryManager<char> manager = new CustomMemoryForTest<char>(a);
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        Span<char> span = manager.Memory.Span;
+                    }
+                }
+            }
+        }
+
+        [Benchmark(InnerIterationCount = InnerCount)]
+        public static void SpanFromObjectMemoryManager()
+        {
+            object o1 = new object();
+            object o2 = new object();
+            object[] a = { o1, o2 };
+            var memory = new Memory<object>(a);
+            MemoryManager<object> manager = new CustomMemoryForTest<object>(a);
+
+            foreach (BenchmarkIteration iteration in Benchmark.Iterations)
+            {
+                using (iteration.StartMeasurement())
+                {
+                    for (int i = 0; i < Benchmark.InnerIterationCount; i++)
+                    {
+                        Span<object> span = manager.Memory.Span;
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/src/System.Memory/tests/Performance/System.Memory.Performance.Tests.csproj b/src/System.Memory/tests/Performance/System.Memory.Performance.Tests.csproj
@@ -10,6 +10,8 @@
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netcoreapp-Release|AnyCPU'" />
   <ItemGroup>
     <Compile Include="Perf.Base64EncodeDecode.cs" />
+    <Compile Include="Perf.Memory.Slice.cs" />
+    <Compile Include="Perf.Memory.Span.cs" />
     <Compile Include="Perf.ReadOnlySequence.Enumerator.cs" />
     <Compile Include="Perf.ReadOnlySequence.First.cs" />
     <Compile Include="Perf.ReadOnlySequence.GetPosition.cs" />
@@ -23,7 +25,6 @@
     <Compile Include="Perf.Span.IndexOfAny.cs" />
     <Compile Include="Perf.Span.SequenceCompareTo.cs" />
     <Compile Include="Perf.Span.StartsWith.cs" />
-    <Compile Include="Perf.MemorySlice.cs" />
     <Compile Include="Perf.Utf8Formatter.cs" />
     <Compile Include="Perf.Utf8Parser.cs" />
     <Compile Include="..\Base64\Base64TestHelper.cs" />

diff --git a/src/System.Memory/tests/ReadOnlyMemory/Span.cs b/src/System.Memory/tests/ReadOnlyMemory/Span.cs
@@ -41,6 +41,22 @@ public static void SpanFromCtorArrayLong()
             ((ReadOnlyMemory<long>)manager.Memory).Span.Validate(91, -92, 93, 94, -95);
         }
 
+        [Fact]
+        public static void SpanFromCtorArrayChar()
+        {
+            char[] a = { '1', '2', '3', '4', '-' };
+            ReadOnlyMemory<char> memory;
+
+            memory = new ReadOnlyMemory<char>(a);
+            memory.Span.Validate('1', '2', '3', '4', '-');
+
+            memory = new ReadOnlyMemory<char>(a, 0, a.Length);
+            memory.Span.Validate('1', '2', '3', '4', '-');
+
+            MemoryManager<char> manager = new CustomMemoryForTest<char>(a);
+            ((ReadOnlyMemory<char>)manager.Memory).Span.Validate('1', '2', '3', '4', '-');
+        }
+
         [Fact]
         public static void SpanFromCtorArrayObject()
         {
@@ -59,6 +75,19 @@ public static void SpanFromCtorArrayObject()
             ((ReadOnlyMemory<object>)manager.Memory).Span.ValidateReferenceType(o1, o2);
         }
 
+        [Fact]
+        public static void SpanFromStringAsMemory()
+        {
+            string a = "1234-";
+            ReadOnlyMemory<char> memory;
+
+            memory = a.AsMemory();
+            memory.Span.Validate('1', '2', '3', '4', '-');
+
+            memory = a.AsMemory(0, a.Length);
+            memory.Span.Validate('1', '2', '3', '4', '-');
+        }
+
         [Fact]
         public static void SpanFromCtorArrayZeroLength()
         {