From 1a4381969afcb7fdf0485aefed59900359cfab47 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 27 Mar 2018 17:05:33 -0700 Subject: [PATCH 1/4] Adding Memory.Pin() to eventually replace Memory.Retain(bool) (#17269) * Adding Memory.Pin() to eventually replace Memory.Retain(bool) * Fix copy/paste error and return default for when object is null. * Fix XML comments. Signed-off-by: dotnet-bot-corefx-mirror --- src/Common/src/CoreLib/System/Memory.cs | 44 ++++++++++++++++++- .../src/CoreLib/System/ReadOnlyMemory.cs | 38 +++++++++++++++- 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/src/Common/src/CoreLib/System/Memory.cs b/src/Common/src/CoreLib/System/Memory.cs index 508ebcb3ee3b..bb2b1557a990 100644 --- a/src/Common/src/CoreLib/System/Memory.cs +++ b/src/Common/src/CoreLib/System/Memory.cs @@ -270,9 +270,49 @@ public Span Span public bool TryCopyTo(Memory destination) => Span.TryCopyTo(destination.Span); /// - /// Returns a handle for the array. - /// If pin is true, the GC will not move the array and hence its address can be taken + /// Creates a handle for the memory. + /// The GC will not move the array until the returned + /// is disposed, enabling taking and using the memory's address. /// + public unsafe MemoryHandle Pin() + { + if (_index < 0) + { + return ((OwnedMemory)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf()); + } + else if (typeof(T) == typeof(char) && _object is string s) + { + // This case can only happen if a ReadOnlyMemory was created around a string + // and then that was cast to a Memory using unsafe / marshaling code. This needs + // to work, however, so that code that uses a single Memory field to store either + // a readable ReadOnlyMemory or a writable Memory can still be pinned and + // used for interop purposes. + GCHandle handle = GCHandle.Alloc(s, GCHandleType.Pinned); +#if FEATURE_PORTABLE_SPAN + void* pointer = Unsafe.Add((void*)handle.AddrOfPinnedObject(), _index); +#else + void* pointer = Unsafe.Add(Unsafe.AsPointer(ref s.GetRawStringData()), _index); +#endif // FEATURE_PORTABLE_SPAN + return new MemoryHandle(null, pointer, handle); + } + else if (_object is T[] array) + { + var handle = GCHandle.Alloc(array, GCHandleType.Pinned); +#if FEATURE_PORTABLE_SPAN + void* pointer = Unsafe.Add((void*)handle.AddrOfPinnedObject(), _index); +#else + void* pointer = Unsafe.Add(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index); +#endif // FEATURE_PORTABLE_SPAN + return new MemoryHandle(null, pointer, handle); + } + return default; + } + + /// [Obsolete, use Pin()] Creates a handle for the memory. + /// + /// If pin is true, the GC will not move the array until the returned + /// is disposed, enabling taking and using the memory's address. + /// public unsafe MemoryHandle Retain(bool pin = false) { MemoryHandle memoryHandle = default; diff --git a/src/Common/src/CoreLib/System/ReadOnlyMemory.cs b/src/Common/src/CoreLib/System/ReadOnlyMemory.cs index 5f3f0e19805e..dca7db3dfd16 100644 --- a/src/Common/src/CoreLib/System/ReadOnlyMemory.cs +++ b/src/Common/src/CoreLib/System/ReadOnlyMemory.cs @@ -226,10 +226,44 @@ public ReadOnlySpan Span /// The span to copy items into. public bool TryCopyTo(Memory destination) => Span.TryCopyTo(destination.Span); - /// Creates a handle for the memory. + /// + /// Creates a handle for the memory. + /// The GC will not move the array until the returned + /// is disposed, enabling taking and using the memory's address. + /// + public unsafe MemoryHandle Pin() + { + if (_index < 0) + { + return ((OwnedMemory)_object).Pin((_index & RemoveOwnedFlagBitMask) * Unsafe.SizeOf()); + } + else if (typeof(T) == typeof(char) && _object is string s) + { + GCHandle handle = GCHandle.Alloc(s, GCHandleType.Pinned); +#if FEATURE_PORTABLE_SPAN + void* pointer = Unsafe.Add((void*)handle.AddrOfPinnedObject(), _index); +#else + void* pointer = Unsafe.Add(Unsafe.AsPointer(ref s.GetRawStringData()), _index); +#endif // FEATURE_PORTABLE_SPAN + return new MemoryHandle(null, pointer, handle); + } + else if (_object is T[] array) + { + var handle = GCHandle.Alloc(array, GCHandleType.Pinned); +#if FEATURE_PORTABLE_SPAN + void* pointer = Unsafe.Add((void*)handle.AddrOfPinnedObject(), _index); +#else + void* pointer = Unsafe.Add(Unsafe.AsPointer(ref array.GetRawSzArrayData()), _index); +#endif // FEATURE_PORTABLE_SPAN + return new MemoryHandle(null, pointer, handle); + } + return default; + } + + /// [Obsolete, use Pin()] Creates a handle for the memory. /// /// If pin is true, the GC will not move the array until the returned - /// is disposed, enabling the memory's address can be taken and used. + /// is disposed, enabling taking and using the memory's address. /// public unsafe MemoryHandle Retain(bool pin = false) { From 3105591f70d6e9f92fca175c1dd886098c8f3c96 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 28 Mar 2018 11:09:42 -0700 Subject: [PATCH 2/4] Nit changes in API diff between .NET Core 2.0 and .NET Core 2.1 (#17288) * 'value' to 'other': bool System.IEquatable.Equals(UIntPtr value); * 'value' to 'other': bool System.IEquatable.Equals(IntPtr value); Signed-off-by: dotnet-bot-corefx-mirror --- src/Common/src/CoreLib/System/IntPtr.cs | 4 ++-- src/Common/src/CoreLib/System/UIntPtr.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Common/src/CoreLib/System/IntPtr.cs b/src/Common/src/CoreLib/System/IntPtr.cs index 684c1389a4ec..44638ddd8ee9 100644 --- a/src/Common/src/CoreLib/System/IntPtr.cs +++ b/src/Common/src/CoreLib/System/IntPtr.cs @@ -81,9 +81,9 @@ public unsafe override bool Equals(Object obj) return false; } - unsafe bool IEquatable.Equals(IntPtr value) + unsafe bool IEquatable.Equals(IntPtr other) { - return _value == value._value; + return _value == other._value; } public unsafe override int GetHashCode() diff --git a/src/Common/src/CoreLib/System/UIntPtr.cs b/src/Common/src/CoreLib/System/UIntPtr.cs index 590722668201..8b2568fde1e5 100644 --- a/src/Common/src/CoreLib/System/UIntPtr.cs +++ b/src/Common/src/CoreLib/System/UIntPtr.cs @@ -77,9 +77,9 @@ public unsafe override bool Equals(Object obj) return false; } - unsafe bool IEquatable.Equals(UIntPtr value) + unsafe bool IEquatable.Equals(UIntPtr other) { - return _value == value._value; + return _value == other._value; } public unsafe override int GetHashCode() From ce84b4dc5af591889fb88ea2de3f95f6cfdd2679 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Thu, 29 Mar 2018 00:11:38 +0100 Subject: [PATCH 3/4] Fix Dictionary CopyTo regression (dotnet/coreclr#17300) Signed-off-by: dotnet-bot-corefx-mirror --- .../System/Collections/Generic/Dictionary.cs | 14 +++++++------- .../src/CoreLib/System/Collections/Generic/List.cs | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs b/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs index 827cc24ac247..f0e892859ed1 100644 --- a/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs +++ b/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs @@ -315,7 +315,7 @@ private void CopyTo(KeyValuePair[] array, int index) { if (entries[i].hashCode >= 0) { - array[index + i] = new KeyValuePair(entries[i].key, entries[i].value); + array[index++] = new KeyValuePair(entries[i].key, entries[i].value); } } } @@ -822,7 +822,7 @@ void ICollection.CopyTo(Array array, int index) { if (entries[i].hashCode >= 0) { - dictEntryArray[index + i] = new DictionaryEntry(entries[i].key, entries[i].value); + dictEntryArray[index++] = new DictionaryEntry(entries[i].key, entries[i].value); } } } @@ -842,7 +842,7 @@ void ICollection.CopyTo(Array array, int index) { if (entries[i].hashCode >= 0) { - objects[index + i] = new KeyValuePair(entries[i].key, entries[i].value); + objects[index++] = new KeyValuePair(entries[i].key, entries[i].value); } } } @@ -1210,7 +1210,7 @@ public void CopyTo(TKey[] array, int index) Entry[] entries = _dictionary._entries; for (int i = 0; i < count; i++) { - if (entries[i].hashCode >= 0) array[index + i] = entries[i].key; + if (entries[i].hashCode >= 0) array[index++] = entries[i].key; } } @@ -1270,7 +1270,7 @@ void ICollection.CopyTo(Array array, int index) { for (int i = 0; i < count; i++) { - if (entries[i].hashCode >= 0) objects[index + i] = entries[i].key; + if (entries[i].hashCode >= 0) objects[index++] = entries[i].key; } } catch (ArrayTypeMismatchException) @@ -1393,7 +1393,7 @@ public void CopyTo(TValue[] array, int index) Entry[] entries = _dictionary._entries; for (int i = 0; i < count; i++) { - if (entries[i].hashCode >= 0) array[index + i] = entries[i].value; + if (entries[i].hashCode >= 0) array[index++] = entries[i].value; } } @@ -1453,7 +1453,7 @@ void ICollection.CopyTo(Array array, int index) { for (int i = 0; i < count; i++) { - if (entries[i].hashCode >= 0) objects[index + i] = entries[i].value; + if (entries[i].hashCode >= 0) objects[index++] = entries[i].value; } } catch (ArrayTypeMismatchException) diff --git a/src/Common/src/CoreLib/System/Collections/Generic/List.cs b/src/Common/src/CoreLib/System/Collections/Generic/List.cs index a5cbf12a658b..6b9f9b45b35c 100644 --- a/src/Common/src/CoreLib/System/Collections/Generic/List.cs +++ b/src/Common/src/CoreLib/System/Collections/Generic/List.cs @@ -163,12 +163,12 @@ public T this[int index] set { - _version++; if ((uint)index >= (uint)_size) { ThrowHelper.ThrowArgumentOutOfRange_IndexException(); } _items[index] = value; + _version++; } } From c517037ed33a116c14c3736adde0597cfb527f83 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Wed, 28 Mar 2018 16:33:56 -0700 Subject: [PATCH 4/4] Use ROSpan.IndexOf as the workhorse for string.IndexOf (#17284) * Use ROSpan.IndexOf as the workhorse for string.IndexOf * Make changes to Span.IndexOf to follow what string.IndexOf did. * Address PR feedback. * Use Unsafe.Read instead of Unsafe.ReadUnaligned. * Remove special casing for count == 0 * Fix up debug assert to use vector count instead of intptr.size * Use size of Vector instead of Vector.Count Signed-off-by: dotnet-bot-corefx-mirror --- .../src/CoreLib/System/SpanHelpers.Char.cs | 128 ++++++++++-------- .../src/CoreLib/System/String.Searching.cs | 116 +--------------- 2 files changed, 73 insertions(+), 171 deletions(-) diff --git a/src/Common/src/CoreLib/System/SpanHelpers.Char.cs b/src/Common/src/CoreLib/System/SpanHelpers.Char.cs index 2033d8b90636..47e83b7b26c5 100644 --- a/src/Common/src/CoreLib/System/SpanHelpers.Char.cs +++ b/src/Common/src/CoreLib/System/SpanHelpers.Char.cs @@ -84,79 +84,87 @@ public static unsafe int IndexOf(ref char searchSpace, char value, int length) { Debug.Assert(length >= 0); - uint uValue = value; // Use uint for comparisons to avoid unnecessary 8->32 extensions - IntPtr index = (IntPtr)0; // Use IntPtr for arithmetic to avoid unnecessary 64->32->64 truncations - IntPtr nLength = (IntPtr)length; -#if !netstandard11 - if (Vector.IsHardwareAccelerated && length >= Vector.Count * 2) + fixed (char* pChars = &searchSpace) { - const int elementsPerByte = sizeof(ushort) / sizeof(byte); - int unaligned = ((int)Unsafe.AsPointer(ref searchSpace) & (Vector.Count - 1)) / elementsPerByte; - nLength = (IntPtr)((Vector.Count - unaligned) & (Vector.Count - 1)); - } - SequentialScan: + char* pCh = pChars; + char* pEndCh = pCh + length; + +#if !netstandard11 + if (Vector.IsHardwareAccelerated && length >= Vector.Count * 2) + { + const int elementsPerByte = sizeof(ushort) / sizeof(byte); + int unaligned = ((int)pCh & (Unsafe.SizeOf>() - 1)) / elementsPerByte; + length = ((Vector.Count - unaligned) & (Vector.Count - 1)); + } + SequentialScan: #endif - while ((byte*)nLength >= (byte*)4) - { - nLength -= 4; - - if (uValue == Unsafe.Add(ref searchSpace, index)) - goto Found; - if (uValue == Unsafe.Add(ref searchSpace, index + 1)) - goto Found1; - if (uValue == Unsafe.Add(ref searchSpace, index + 2)) - goto Found2; - if (uValue == Unsafe.Add(ref searchSpace, index + 3)) - goto Found3; - - index += 4; - } + while (length >= 4) + { + length -= 4; + + if (*pCh == value) + goto Found; + if (*(pCh + 1) == value) + goto Found1; + if (*(pCh + 2) == value) + goto Found2; + if (*(pCh + 3) == value) + goto Found3; + + pCh += 4; + } - while ((byte*)nLength > (byte*)0) - { - nLength -= 1; + while (length > 0) + { + length -= 1; - if (uValue == Unsafe.Add(ref searchSpace, index)) - goto Found; + if (*pCh == value) + goto Found; - index += 1; - } + pCh += 1; + } #if !netstandard11 - if (Vector.IsHardwareAccelerated && ((int)(byte*)index < length)) - { - nLength = (IntPtr)((length - (int)(byte*)index) & ~(Vector.Count - 1)); + // We get past SequentialScan only if IsHardwareAccelerated is true. However, we still have the redundant check to allow + // the JIT to see that the code is unreachable and eliminate it when the platform does not have hardware accelerated. + if (Vector.IsHardwareAccelerated && pCh < pEndCh) + { + length = (int)((pEndCh - pCh) & ~(Vector.Count - 1)); - // Get comparison Vector - Vector vComparison = new Vector(value); + // Get comparison Vector + Vector vComparison = new Vector(value); - while ((byte*)nLength > (byte*)index) - { - var vMatches = Vector.Equals(vComparison, Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref searchSpace, index)))); - if (Vector.Zero.Equals(vMatches)) + while (length > 0) { - index += Vector.Count; - continue; + // Using Unsafe.Read instead of ReadUnaligned since the search space is pinned and pCh is always vector aligned + Debug.Assert(((int)pCh & (Unsafe.SizeOf>() - 1)) == 0); + Vector vMatches = Vector.Equals(vComparison, Unsafe.Read>(pCh)); + if (Vector.Zero.Equals(vMatches)) + { + pCh += Vector.Count; + length -= Vector.Count; + continue; + } + // Find offset of first match + return (int)(pCh - pChars) + LocateFirstFoundChar(vMatches); } - // Find offset of first match - return (int)(byte*)index + LocateFirstFoundChar(vMatches); - } - if ((int)(byte*)index < length) - { - nLength = (IntPtr)(length - (int)(byte*)index); - goto SequentialScan; + if (pCh < pEndCh) + { + length = (int)(pEndCh - pCh); + goto SequentialScan; + } } - } #endif - return -1; - Found: // Workaround for https://github.com/dotnet/coreclr/issues/13549 - return (int)(byte*)index; - Found1: - return (int)(byte*)(index + 1); - Found2: - return (int)(byte*)(index + 2); - Found3: - return (int)(byte*)(index + 3); + return -1; + Found3: + pCh++; + Found2: + pCh++; + Found1: + pCh++; + Found: + return (int)(pCh - pChars); + } } #if !netstandard11 diff --git a/src/Common/src/CoreLib/System/String.Searching.cs b/src/Common/src/CoreLib/System/String.Searching.cs index cc6e218d8d17..a4e5bbb62e8a 100644 --- a/src/Common/src/CoreLib/System/String.Searching.cs +++ b/src/Common/src/CoreLib/System/String.Searching.cs @@ -35,10 +35,7 @@ public bool Contains(char value, StringComparison comparisonType) // Returns the index of the first occurrence of a specified character in the current instance. // The search starts at startIndex and runs thorough the next count characters. // - public int IndexOf(char value) - { - return IndexOf(value, 0, this.Length); - } + public int IndexOf(char value) => SpanHelpers.IndexOf(ref _firstChar, value, Length); public int IndexOf(char value, int startIndex) { @@ -74,122 +71,19 @@ public int IndexOf(char value, StringComparison comparisonType) public unsafe int IndexOf(char value, int startIndex, int count) { + // These bounds checks are redundant since AsSpan does them already, but are required + // so that the correct parameter name is thrown as part of the exception. if ((uint)startIndex > (uint)Length) throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_Index); if ((uint)count > (uint)(Length - startIndex)) throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_Count); - fixed (char* pChars = &_firstChar) - { - char* pCh = pChars + startIndex; - char* pEndCh = pCh + count; - - if (Vector.IsHardwareAccelerated && count >= Vector.Count * 2) - { - unchecked - { - const int elementsPerByte = sizeof(ushort) / sizeof(byte); - int unaligned = ((int)pCh & (Vector.Count - 1)) / elementsPerByte; - count = ((Vector.Count - unaligned) & (Vector.Count - 1)); - } - } - SequentialScan: - while (count >= 4) - { - if (*pCh == value) goto ReturnIndex; - if (*(pCh + 1) == value) goto ReturnIndex1; - if (*(pCh + 2) == value) goto ReturnIndex2; - if (*(pCh + 3) == value) goto ReturnIndex3; - - count -= 4; - pCh += 4; - } - - while (count > 0) - { - if (*pCh == value) - goto ReturnIndex; - - count--; - pCh++; - } - - if (pCh < pEndCh) - { - count = (int)((pEndCh - pCh) & ~(Vector.Count - 1)); - // Get comparison Vector - Vector vComparison = new Vector(value); - while (count > 0) - { - var vMatches = Vector.Equals(vComparison, Unsafe.ReadUnaligned>(pCh)); - if (Vector.Zero.Equals(vMatches)) - { - pCh += Vector.Count; - count -= Vector.Count; - continue; - } - // Find offset of first match - return (int)(pCh - pChars) + LocateFirstFoundChar(vMatches); - } - - if (pCh < pEndCh) - { - unchecked - { - count = (int)(pEndCh - pCh); - } - goto SequentialScan; - } - } - - return -1; + int result = SpanHelpers.IndexOf(ref Unsafe.Add(ref _firstChar, startIndex), value, count); - ReturnIndex3: pCh++; - ReturnIndex2: pCh++; - ReturnIndex1: pCh++; - ReturnIndex: - return (int)(pCh - pChars); - } + return result == -1 ? result : result + startIndex; } - // Vector sub-search adapted from https://github.com/aspnet/KestrelHttpServer/pull/1138 - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static int LocateFirstFoundChar(Vector match) - { - var vector64 = Vector.AsVectorUInt64(match); - ulong candidate = 0; - int i = 0; - // Pattern unrolled by jit https://github.com/dotnet/coreclr/pull/8001 - for (; i < Vector.Count; i++) - { - candidate = vector64[i]; - if (candidate != 0) - { - break; - } - } - - // Single LEA instruction with jitted const (using function result) - return i * 4 + LocateFirstFoundChar(candidate); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static int LocateFirstFoundChar(ulong match) - { - unchecked - { - // Flag least significant power of two bit - var powerOfTwoFlag = match ^ (match - 1); - // Shift all powers of two into the high byte and extract - return (int)((powerOfTwoFlag * XorPowerOfTwoToHighChar) >> 49); - } - } - - private const ulong XorPowerOfTwoToHighChar = (0x03ul | - 0x02ul << 16 | - 0x01ul << 32) + 1; - // Returns the index of the first occurrence of any specified character in the current instance. // The search starts at startIndex and runs to startIndex + count - 1. //