MemoryPoolIterator feedback

global.json

@@ -1,3 +1,3 @@
 {
-    "projects": [ "src" ]
+    "projects": ["src"]
 }

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs

@@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
 {
     public abstract partial class Frame : IFrameControl
     {
-        // byte consts don't have a data type annotation so we pre-cast them
+        // byte types don't have a data type annotation so we pre-cast them; to avoid in-place casts
         private const byte ByteCR = (byte)'\r';
         private const byte ByteLF = (byte)'\n';
         private const byte ByteColon = (byte)':';

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/MemoryPoolIterator.cs

@@ -774,10 +774,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static int LocateFirstFoundByte(Vector<byte> byteEquals)
         {
-            var vector64 = Vector.AsVectorInt64(byteEquals);
+            var vector64 = Vector.AsVectorUInt64(byteEquals);
-            long longValue = 0;
+            ulong longValue = 0;
             var i = 0;
-            for (; i < Vector<long>.Count; i++)
+            // Pattern unrolled by jit https://github.com/dotnet/coreclr/pull/8001
+            for (; i < Vector<ulong>.Count; i++)
             {
                 longValue = vector64[i];
                 if (longValue == 0) continue;
@@ -785,7 +786,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
             }
 
             // Flag least significant power of two bit
-            var powerOfTwoFlag = (ulong)(longValue ^ (longValue - 1));
+            var powerOfTwoFlag = (longValue ^ (longValue - 1));
             // Shift all powers of two into the high byte and extract
             var foundByteIndex = (int)((powerOfTwoFlag * _xorPowerOfTwoToHighByte) >> 57);
             // Single LEA instruction with jitted const (using function result)
@@ -803,7 +804,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
             var block = _block;
             if (block == null)
             {
-                return false;
+                ThrowInvalidOperationException_PutPassedEndOfBlock();
             }
 
             var index = _index;
@@ -817,7 +818,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
                 return true;
             }
 
-            return wasLastBlock ? false : PutMultiBlock(data);
+            if (wasLastBlock)
+            {
+                ThrowInvalidOperationException_PutPassedEndOfBlock();
+            }
+
+            return PutMultiBlock(data);
         }
 
         [MethodImpl(MethodImplOptions.NoInlining)]
@@ -841,6 +847,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
                 }
                 if (wasLastBlock)
                 {
+                    ThrowInvalidOperationException_PutPassedEndOfBlock();
                     return false;
                 }
             } while (true);
@@ -848,13 +855,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
             return true;
         }
 
+        private static void ThrowInvalidOperationException_PutPassedEndOfBlock()
+        {
+            throw new InvalidOperationException("Attempted to put passed end of block.");
+        }
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public int GetLength(MemoryPoolIterator end)
         {
             var block = _block;
             if (block == null || end.IsDefault)
             {
-                return -1;
+                ThrowInvalidOperationException_GetLengthNullBlock();
             }
 
             if (block == end._block)
@@ -865,6 +877,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
             return GetLengthMultiBlock(ref end);
         }
 
+        private static void ThrowInvalidOperationException_GetLengthNullBlock()
+        {
+            throw new InvalidOperationException("Attempted GetLength of non existent block.");
+        }
+
         [MethodImpl(MethodImplOptions.NoInlining)]
         public int GetLengthMultiBlock(ref MemoryPoolIterator end)
         {
@@ -1234,7 +1251,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
             // Vector<byte> .ctor doesn't become an intrinsic due to detection issue
             // However this does cause it to become an intrinsic (with additional multiply and reg->reg copy)
             // https://github.com/dotnet/coreclr/issues/7459#issuecomment-253965670
-            return Vector.AsVectorByte(new Vector<ulong>(vectorByte * 0x0101010101010101ul));
+            return Vector.AsVectorByte(new Vector<uint>(vectorByte * 0x01010101u));
         }
 
     }

test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs

@@ -8,7 +8,6 @@ using BenchmarkDotNet.Attributes;
 using Microsoft.AspNetCore.Server.Kestrel.Internal;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
-using Microsoft.AspNetCore.Server.KestrelTests.TestHelpers;
 using Microsoft.AspNetCore.Testing;
 using RequestLineStatus = Microsoft.AspNetCore.Server.Kestrel.Internal.Http.Frame.RequestLineStatus;
 

test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolIteratorTests.cs

@@ -154,7 +154,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             }
 
             // Can't put anything by the end
-            Assert.False(head.Put(0xFF));
+            Assert.ThrowsAny<InvalidOperationException>(() => head.Put(0xFF));
 
             for (var i = 0; i < 4; ++i)
             {
@@ -979,7 +979,6 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             var end = default(MemoryPoolIterator);
 
             Assert.False(default(MemoryPoolIterator).TryPeekLong(out longValue));
-            Assert.False(default(MemoryPoolIterator).Put(byteCr));
             Assert.Null(default(MemoryPoolIterator).GetAsciiString(ref end));
             Assert.Null(default(MemoryPoolIterator).GetUtf8String(ref end));
             // Assert.Equal doesn't work for default(ArraySegments)
@@ -995,7 +994,8 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
 
             default(MemoryPoolIterator).CopyFrom(default(ArraySegment<byte>));
             default(MemoryPoolIterator).CopyFromAscii("");
-            Assert.Equal(default(MemoryPoolIterator).GetLength(end), -1);
+            Assert.ThrowsAny<InvalidOperationException>(() => default(MemoryPoolIterator).Put(byteCr));
+            Assert.ThrowsAny<InvalidOperationException>(() => default(MemoryPoolIterator).GetLength(end));
             Assert.ThrowsAny<InvalidOperationException>(() => default(MemoryPoolIterator).Skip(1));
         }
 

test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs

@@ -10,11 +10,11 @@ using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Server.Kestrel;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
-using Microsoft.AspNetCore.Server.KestrelTests.TestHelpers;
 using Microsoft.Extensions.Internal;
 using Moq;
 using Xunit;
 using Xunit.Sdk;
+using Microsoft.AspNetCore.Testing;
 
 namespace Microsoft.AspNetCore.Server.KestrelTests
 {

test/shared/MockConnection.cs

@@ -8,7 +8,7 @@ using Microsoft.AspNetCore.Server.Kestrel;
 using Microsoft.AspNetCore.Server.Kestrel.Internal;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
 
-namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers
+namespace Microsoft.AspNetCore.Testing
 {
     public class MockConnection : Connection, IDisposable
     {

test/shared/SocketInputExtensions.cs

@@ -4,7 +4,7 @@
 using System;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
 
-namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers
+namespace Microsoft.AspNetCore.Testing
 {
     public static class SocketInputExtensions
     {