From 1dfe097dc0201564e61461731058569a20517183 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Mon, 9 Apr 2018 22:00:13 -0700 Subject: [PATCH] Fix MemoryManager ctor, add unit and perf tests, and improve performance (#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 --- src/Common/src/CoreLib/System/Memory.cs | 13 +- .../src/CoreLib/System/ReadOnlyMemory.cs | 2 + src/System.Memory/src/System/ThrowHelper.cs | 3 +- .../tests/Memory/MemoryManager.cs | 27 +++ src/System.Memory/tests/Memory/Span.cs | 30 ++++ ...rf.MemorySlice.cs => Perf.Memory.Slice.cs} | 0 .../tests/Performance/Perf.Memory.Span.cs | 166 ++++++++++++++++++ .../System.Memory.Performance.Tests.csproj | 3 +- .../tests/ReadOnlyMemory/Span.cs | 29 +++ 9 files changed, 264 insertions(+), 9 deletions(-) rename src/System.Memory/tests/Performance/{Perf.MemorySlice.cs => Perf.Memory.Slice.cs} (100%) create mode 100644 src/System.Memory/tests/Performance/Perf.Memory.Span.cs diff --git a/src/Common/src/CoreLib/System/Memory.cs b/src/Common/src/CoreLib/System/Memory.cs index 6eb5af665d18..3ccb76b2e5d1 100644 --- a/src/Common/src/CoreLib/System/Memory.cs +++ b/src/Common/src/CoreLib/System/Memory.cs @@ -120,7 +120,9 @@ public Memory(T[] array, int start, int length) /// The memory manager. /// The index at which to begin the memory. /// The number of items in the memory. - /// Returns default when is null. + /// + /// Thrown when is null. + /// /// /// Thrown when the specified or end index is not in the range (<0 or >=Length). /// @@ -128,12 +130,7 @@ public Memory(T[] array, int start, int length) public Memory(MemoryManager 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 Span if (_index < 0) { Debug.Assert(_length >= 0); + Debug.Assert(_object != null); return ((MemoryManager)_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)_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 index ca0e7d888e4d..3e7884528edb 100644 --- a/src/Common/src/CoreLib/System/ReadOnlyMemory.cs +++ b/src/Common/src/CoreLib/System/ReadOnlyMemory.cs @@ -187,6 +187,7 @@ public ReadOnlySpan Span if (_index < 0) { Debug.Assert(_length >= 0); + Debug.Assert(_object != null); return ((MemoryManager)_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)_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 index 5a854a49b3a1..40683539380b 100644 --- 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 index 4f6f2461cf9c..5a6c675c72e2 100644 --- 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 managerInt = default; + Assert.Throws(() => new Memory(managerInt, 0, 0)); + + managerInt = null; + Assert.Throws(() => new Memory(managerInt, 0, 0)); + + MemoryManager managerObject = default; + Assert.Throws(() => new Memory(managerObject, 0, 0)); + } + + [Fact] + public static void MemoryManagerCtorInvalid() + { + int[] a = { 91, 92, -93, 94 }; + MemoryManager manager = new CustomMemoryForTest(a); + Assert.Throws(() => new Memory(manager, 0, -1)); + Assert.Throws(() => new Memory(manager, -1, 0)); + Assert.Throws(() => new Memory(manager, -1, -1)); + Assert.Throws(() => new Memory(manager, -1, -1)); + Assert.Throws(() => new Memory(manager, 0, a.Length + 1)); + Assert.Throws(() => new Memory(manager, a.Length + 1, 0)); + Assert.Throws(() => new Memory(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 index 55ed001b3d8a..55956dd6ab31 100644 --- 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 memory; + + memory = new Memory(a); + memory.Span.Validate('1', '2', '3', '4', '-'); + + memory = new Memory(a, 0, a.Length); + memory.Span.Validate('1', '2', '3', '4', '-'); + + MemoryManager manager = new CustomMemoryForTest(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 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/src/System.Memory/tests/Performance/Perf.MemorySlice.cs b/src/System.Memory/tests/Performance/Perf.Memory.Slice.cs similarity index 100% rename from src/System.Memory/tests/Performance/Perf.MemorySlice.cs rename to src/System.Memory/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 new file mode 100644 index 000000000000..f2db92e0f4cf --- /dev/null +++ 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 memory = default; + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + Span span = memory.Span; + } + } + } + } + + [Benchmark(InnerIterationCount = InnerCount)] + public static void SpanFromIntegerArrayBackedMemory() + { + int[] a = { 91, 92, -93, 94 }; + var memory = new Memory(a); + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + Span span = memory.Span; + } + } + } + } + + [Benchmark(InnerIterationCount = InnerCount)] + public static void SpanFromCharArrayBackedMemory() + { + char[] a = "9192-9394".ToCharArray(); + var memory = new Memory(a); + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + Span 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(a); + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + Span span = memory.Span; + } + } + } + } + + [Benchmark(InnerIterationCount = InnerCount)] + public static void SpanFromStringBackedMemory() + { + string a = "9192-9394"; + ReadOnlyMemory memory = a.AsMemory(); + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + ReadOnlySpan span = memory.Span; + } + } + } + } + + [Benchmark(InnerIterationCount = InnerCount)] + public static void SpanFromIntegerMemoryManager() + { + int[] a = { 91, 92, -93, 94 }; + var memory = new Memory(a); + MemoryManager manager = new CustomMemoryForTest(a); + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + Span span = manager.Memory.Span; + } + } + } + } + + [Benchmark(InnerIterationCount = InnerCount)] + public static void SpanFromCharMemoryManager() + { + char[] a = "9192-9394".ToCharArray(); + var memory = new Memory(a); + MemoryManager manager = new CustomMemoryForTest(a); + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + Span 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(a); + MemoryManager manager = new CustomMemoryForTest(a); + + foreach (BenchmarkIteration iteration in Benchmark.Iterations) + { + using (iteration.StartMeasurement()) + { + for (int i = 0; i < Benchmark.InnerIterationCount; i++) + { + Span 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 index edb6ffe73c93..28249af672c5 100644 --- 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 @@ + + @@ -23,7 +25,6 @@ - diff --git a/src/System.Memory/tests/ReadOnlyMemory/Span.cs b/src/System.Memory/tests/ReadOnlyMemory/Span.cs index 50ac96ee79a9..728c65e2e107 100644 --- 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)manager.Memory).Span.Validate(91, -92, 93, 94, -95); } + [Fact] + public static void SpanFromCtorArrayChar() + { + char[] a = { '1', '2', '3', '4', '-' }; + ReadOnlyMemory memory; + + memory = new ReadOnlyMemory(a); + memory.Span.Validate('1', '2', '3', '4', '-'); + + memory = new ReadOnlyMemory(a, 0, a.Length); + memory.Span.Validate('1', '2', '3', '4', '-'); + + MemoryManager manager = new CustomMemoryForTest(a); + ((ReadOnlyMemory)manager.Memory).Span.Validate('1', '2', '3', '4', '-'); + } + [Fact] public static void SpanFromCtorArrayObject() { @@ -59,6 +75,19 @@ public static void SpanFromCtorArrayObject() ((ReadOnlyMemory)manager.Memory).Span.ValidateReferenceType(o1, o2); } + [Fact] + public static void SpanFromStringAsMemory() + { + string a = "1234-"; + ReadOnlyMemory 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() {