Refactor implementation to use the four-fields approach (#23)

* intermediate non-compiling commit

* well, it compiles...

* fix all existing tests

* move SequenceSegment (and related types) out of Block.cs

* c# 8.0 breaks benchmarkdotnet

* clean up some junk in the benchmarks

* move the enumerator mess out of the main Sequence.cs; better pinned usage in Reference<T>

* add all the benchmarks as tests; fix glitch re end-of-block allocations

* moar scenarios; lots of fixes

* fix GetReference bug; rationalize APIs to take Memory<T>

* lots of test and perf fixups

* remove some indirections that are impacting perf

* fix the unsafe tests that were breaking the test rig

Benchmarks.cmd

@@ -1,2 +1,2 @@
 @ECHO OFF
-dotnet run -p .\tests\Benchmark\ -c Release -f net472 --runtimes net472 netcoreapp2.0 netcoreapp2.1 netcoreapp2.2 -m
+dotnet run -p .\tests\Benchmark\ -c Release -f netcoreapp2.2 --runtimes net472 netcoreapp2.2 -m

Toys/PipeTextReader.cs

@@ -20,7 +20,7 @@ public sealed class PipeTextReader : TextReader
         private readonly PipeReader _reader;
         private readonly Decoder _decoder;
         private readonly Encoding _encoding;
-        private readonly int _maxBytesPerChar;
+        // private readonly int _maxBytesPerChar;
         private readonly ReadOnlyMemory<byte> _lineFeed;
 #if !SOCKET_STREAM_BUFFERS
         private readonly ReadOnlyMemory<byte> _preamble;
@@ -75,7 +75,7 @@ private PipeTextReader(PipeReader reader, Encoding encoding, bool closeReader)
                 _crLen = encoding.GetByteCount("\r");
                 _lfLen = _lineFeed.Length;
             }
-            _maxBytesPerChar = encoding.GetMaxByteCount(1); // over-reports, but... meh
+            // _maxBytesPerChar = encoding.GetMaxByteCount(1); // over-reports, but... meh
 
 
 #if SOCKET_STREAM_BUFFERS
@@ -542,7 +542,7 @@ private int ReadSingleChar(bool peek)
             var decoder = GetDecoder();
             if(buffer.IsSingleSegment)
             {
-                decoder.Convert(buffer.First.Span, chars, false, out int bytesUsed, out int charsUsed, out bool completed);
+                decoder.Convert(buffer.First.Span, chars, false, out int bytesUsed, out int charsUsed, out _);
                 if(charsUsed == 1)
                 {
                     _reader.AdvanceTo(peek ? buffer.Start : buffer.GetPosition(bytesUsed), buffer.End);
@@ -554,7 +554,7 @@ private int ReadSingleChar(bool peek)
                 int totalBytes = 0;
                 foreach(var segment in buffer)
                 {
-                    decoder.Convert(segment.Span, chars, false, out int bytesUsed, out int charsUsed, out bool completed);
+                    decoder.Convert(segment.Span, chars, false, out int bytesUsed, out int charsUsed, out _);
                     totalBytes += bytesUsed;
                     if (charsUsed == 1)
                     {

Toys/PipeTextWriter.cs

@@ -47,11 +47,13 @@ public override string NewLine
             get => _newLine;
             set => _newLine = value ?? ""; // if someone tries to set to null, assume they meant empty
         }
-        private Encoder GetEncoder()
-        {
-            _encoder.Reset();
-            return _encoder;
-        }
+
+        //private Encoder GetEncoder()
+        //{
+        //    _encoder.Reset();
+        //    return _encoder;
+        //}
+
         /// <summary>
         /// Create a new instance of a PipeTextWriter
         /// </summary>
@@ -339,7 +341,7 @@ private static int WriteImpl(PipeWriter writer, ReadOnlySpan<char> chars, Encodi
             if (!completed)
             {
                 var bytes = writer.GetSpan(10);
-                encoder.Convert(chars, bytes, true, out int charsUsed, out int bytesUsed, out completed);
+                encoder.Convert(chars, bytes, true, out _, out int bytesUsed, out completed);
                 Debug.Assert(completed);
                 writer.Advance(bytesUsed);
                 totalBytesUsed += bytesUsed;

src/Pipelines.Sockets.Unofficial/Arenas/Allocator.cs

@@ -105,15 +105,14 @@ public override IMemoryOwner<T> Allocate(int length)
         private unsafe sealed class PinnedArray : MemoryManager<T>, IPinnedMemoryOwner<T>
         {
             private T[] _array;
-            private readonly int _length;
             private readonly ArrayPool<T> _pool;
             private GCHandle _pin;
             private T* _ptr;
             public PinnedArray(ArrayPool<T> pool, T[] array)
             {
                 _pool = pool;
                 _array = array;
-                _length = array.Length;
+                Length = array.Length;
                 _pin = GCHandle.Alloc(array, GCHandleType.Pinned);
                 _ptr = (T*)_pin.AddrOfPinnedObject().ToPointer();
             }
@@ -135,7 +134,9 @@ protected override void Dispose(bool disposing)
                 }
             }
 
-            public override Span<T> GetSpan() => new Span<T>(_ptr, _length);
+
+            public int Length { get; }
+            public override Span<T> GetSpan() => new Span<T>(_ptr, Length);
 
             public override MemoryHandle Pin(int elementIndex = 0) => new MemoryHandle(_ptr + elementIndex);
 
@@ -189,14 +190,14 @@ private sealed class OwnedPointer : MemoryManager<T>, IPinnedMemoryOwner<T>
             ~OwnedPointer() => Dispose(false);
 
             private T* _ptr;
-            private readonly int _length;
 
+            public int Length { get; }
             void* IPinnedMemoryOwner<T>.Origin => _ptr;
 
             public OwnedPointer(int length)
-                => _ptr = (T*)Marshal.AllocHGlobal((_length = length) * sizeof(T)).ToPointer();
+                => _ptr = (T*)Marshal.AllocHGlobal((Length = length) * sizeof(T)).ToPointer();
 
-            public override Span<T> GetSpan() => new Span<T>(_ptr, _length);
+            public override Span<T> GetSpan() => new Span<T>(_ptr, Length);
 
             public override MemoryHandle Pin(int elementIndex = 0)
                 => new MemoryHandle(_ptr + elementIndex);

src/Pipelines.Sockets.Unofficial/Arenas/Arena.cs

@@ -110,6 +110,7 @@ public abstract class OwnedArena<T> : IArena
         /// <summary>
         /// Allocate a block of data
         /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public abstract Sequence<T> Allocate(int length);
 
         /// <summary>
@@ -195,9 +196,17 @@ public NonPaddedBlittableOwnedArena(Arena parent) : base(parent)
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public override Sequence<TTo> Allocate(int length)
         {
-            var fromParent = _arena.Allocate(length);
-            var block = (Block<TFrom>)fromParent.GetSegmentAndOffset(out var offset);
-            return new Sequence<TTo>(offset, length, MapBlock(block.SegmentIndex));
+            var fromParent = _arena.AllocateRetainingSegmentData(length);
+
+            if (!fromParent.TryGetSegments(out var startSeq, out var endSeq, out var startOffset, out var endOffset))
+                Throw.InvalidOperation("The segment data was not available");
+
+            var startMapped = MapBlock(((Block<TFrom>)startSeq).SegmentIndex);
+            var endMapped = ReferenceEquals(startSeq, endSeq) ? startMapped : MapBlock(((Block<TFrom>)endSeq).SegmentIndex);
+
+            return new Sequence<TTo>(
+                startMapped, endMapped,
+                startOffset, endOffset);
         }
     }
 
@@ -229,7 +238,9 @@ public override Sequence<T> Allocate(int length)
             // create our result, since we know the details
             Debug.Assert(arena.AllocatedCurrentBlock % Unsafe.SizeOf<T>() == 0, "should be aligned to T");
             var mappedBlock = MapBlock(arena.CurrentBlock.SegmentIndex);
-            var result = new Sequence<T>(arena.AllocatedCurrentBlock / Unsafe.SizeOf<T>(), length, mappedBlock);
+
+            var startBlock = mappedBlock;
+            var startOffset = arena.AllocatedCurrentBlock / Unsafe.SizeOf<T>();
 
             // now we need to allocate and map the rest of the pages, taking care to
             // allow for padding at the end of pages
@@ -262,13 +273,12 @@ public override Sequence<T> Allocate(int length)
                 }
             }
 
-            // make sure that we've mapped as far as the end; we don't actually need the result - we
-            // just need to know that it has happened, as this connects the chain
-            MapBlock(arena.CurrentBlock.SegmentIndex);
+            // make sure that we've mapped as far as the end
+            mappedBlock = MapBlock(arena.CurrentBlock.SegmentIndex);
 
             Debug.Assert(length == 0);
 
-            return result;
+            return new Sequence<T>(startBlock, mappedBlock, startOffset, arena.AllocatedCurrentBlock / Unsafe.SizeOf<T>());
         }
     }
 
@@ -426,10 +436,11 @@ OwnedArena<T> Create()
 
         internal object GetAllocator<T>() => GetArena<T>().GetAllocator();
 
-        internal sealed class MappedSegment<TFrom, TTo> : SequenceSegment<TTo>
+        internal sealed class MappedSegment<TFrom, TTo> : SequenceSegment<TTo>, IPinnedMemoryOwner<TTo>
             where TFrom : unmanaged
             where TTo : unmanaged
         {
+            public unsafe void* Origin { get; }
             public Block<TFrom> Underlying { get; }
 
             protected override Type GetUnderlyingType() => typeof(TFrom);
@@ -442,46 +453,51 @@ internal sealed class MappedSegment<TFrom, TTo> : SequenceSegment<TTo>
             protected override long ByteOffset => _byteOffset;
 #endif
 
-            public unsafe MappedSegment(MappedSegment<TFrom, TTo> previous, Block<TFrom> underlying)
+            static unsafe Memory<TTo> CreateMapped(Block<TFrom> underlying, out void* origin)
             {
-                Underlying = underlying;
                 MemoryManager<TTo> mapped;
+                origin = null;
                 if (underlying.Allocation is IPinnedMemoryOwner<TFrom> rooted && rooted.Origin != null)
                 {   // in this case, we can just cheat like crazy
-                    mapped = new PinnedConvertingMemoryManager(rooted);
+                    var x = new PinnedConvertingMemoryManager(rooted);
+                    origin = x.Origin;
+                    mapped = x;
                 }
                 else
                 {   // need to do everything properly; slower, but it'll work
                     mapped = new ConvertingMemoryManager(underlying.Allocation);
                 }
-                Memory = mapped.Memory;
-
+                return mapped.Memory;
+            }
+            public unsafe MappedSegment(MappedSegment<TFrom, TTo> previous, Block<TFrom> underlying)
+                : base(CreateMapped(underlying, out void* origin), previous)
+            {
+                Origin = origin;
+                Underlying = underlying;
 #if DEBUG
                 _byteCount = underlying.Length * Unsafe.SizeOf<TFrom>();
-#endif
                 if (previous != null)
                 {   // we can't use "underlying" for this, because of padding etc
-                    RunningIndex = previous.RunningIndex + previous.Length;
-#if DEBUG
                     _byteOffset = previous.ByteOffset + previous._byteCount;
-#endif
-                    previous.Next = this;
                 }
+#endif
             }
 
             sealed unsafe class PinnedConvertingMemoryManager : MemoryManager<TTo>, IPinnedMemoryOwner<TTo>
             {
                 private readonly TTo* _origin;
-                private readonly int _length;
+
                 public PinnedConvertingMemoryManager(IPinnedMemoryOwner<TFrom> rooted)
                 {
                     _origin = (TTo*)rooted.Origin;
-                    _length = MemoryMarshal.Cast<TFrom, TTo>(rooted.Memory.Span).Length;
+                    Length = MemoryMarshal.Cast<TFrom, TTo>(rooted.Memory.Span).Length;
                 }
 
-                void* IPinnedMemoryOwner<TTo>.Origin => _origin;
+                public void* Origin => _origin;
+
+                public override Span<TTo> GetSpan() => new Span<TTo>(_origin, Length);
 
-                public override Span<TTo> GetSpan() => new Span<TTo>(_origin, _length);
+                public int Length { get; }
 
                 public override MemoryHandle Pin(int elementIndex = 0) => new MemoryHandle(_origin + elementIndex);