From 2c649b2409635faea572203ee46afe50b21d42d9 Mon Sep 17 00:00:00 2001 From: Jeremy Kuhne Date: Thu, 24 Jan 2019 15:57:18 -0800 Subject: [PATCH 1/2] Use .NET Core SequenceReader Remove BufferReader and use SequenceReader which now ships in CoreFX. This is related to https://github.com/aspnet/KestrelHttpServer/pull/3068 which builds on the functionality added to the reader. --- .../Kestrel/Core/src/Internal/BufferReader.cs | 153 --------- .../src/Internal/Http/Http1MessageBody.cs | 15 +- .../Core/src/Internal/Http/HttpParser.cs | 25 +- .../Kestrel/Core/test/BufferReaderTests.cs | 300 ------------------ 4 files changed, 19 insertions(+), 474 deletions(-) delete mode 100644 src/Servers/Kestrel/Core/src/Internal/BufferReader.cs delete mode 100644 src/Servers/Kestrel/Core/test/BufferReaderTests.cs diff --git a/src/Servers/Kestrel/Core/src/Internal/BufferReader.cs b/src/Servers/Kestrel/Core/src/Internal/BufferReader.cs deleted file mode 100644 index 3fb844addb4e..000000000000 --- a/src/Servers/Kestrel/Core/src/Internal/BufferReader.cs +++ /dev/null @@ -1,153 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Runtime.CompilerServices; - -namespace System.Buffers -{ - internal ref struct BufferReader - { - private ReadOnlySpan _currentSpan; - private int _index; - - private ReadOnlySequence _sequence; - private SequencePosition _currentSequencePosition; - private SequencePosition _nextSequencePosition; - - private int _consumedBytes; - private bool _end; - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public BufferReader(in ReadOnlySequence buffer) - { - _index = 0; - _consumedBytes = 0; - _sequence = buffer; - _currentSequencePosition = _sequence.Start; - _nextSequencePosition = _currentSequencePosition; - - if (_sequence.TryGet(ref _nextSequencePosition, out var memory, true)) - { - _end = false; - _currentSpan = memory.Span; - if (_currentSpan.Length == 0) - { - // No space in first span, move to one with space - MoveNext(); - } - } - else - { - // No space in any spans and at end of sequence - _end = true; - _currentSpan = default; - } - } - - public bool End => _end; - - public int CurrentSegmentIndex => _index; - - public SequencePosition Position => _sequence.GetPosition(_index, _currentSequencePosition); - - public ReadOnlySpan CurrentSegment => _currentSpan; - - public ReadOnlySpan UnreadSegment => _currentSpan.Slice(_index); - - public int ConsumedBytes => _consumedBytes; - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public int Peek() - { - if (_end) - { - return -1; - } - return _currentSpan[_index]; - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public int Read() - { - if (_end) - { - return -1; - } - - var value = _currentSpan[_index]; - _index++; - _consumedBytes++; - - if (_index >= _currentSpan.Length) - { - MoveNext(); - } - - return value; - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private void MoveNext() - { - var previous = _nextSequencePosition; - while (_sequence.TryGet(ref _nextSequencePosition, out var memory, true)) - { - _currentSequencePosition = previous; - _currentSpan = memory.Span; - _index = 0; - if (_currentSpan.Length > 0) - { - return; - } - } - _end = true; - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Advance(int byteCount) - { - if (!_end && byteCount > 0 && (_index + byteCount) < _currentSpan.Length) - { - _consumedBytes += byteCount; - _index += byteCount; - } - else - { - AdvanceNext(byteCount); - } - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private void AdvanceNext(int byteCount) - { - if (byteCount < 0) - { - BuffersThrowHelper.ThrowArgumentOutOfRangeException(BuffersThrowHelper.ExceptionArgument.length); - } - - _consumedBytes += byteCount; - - while (!_end && byteCount > 0) - { - if ((_index + byteCount) < _currentSpan.Length) - { - _index += byteCount; - byteCount = 0; - break; - } - - var remaining = (_currentSpan.Length - _index); - - _index += remaining; - byteCount -= remaining; - - MoveNext(); - } - - if (byteCount > 0) - { - BuffersThrowHelper.ThrowArgumentOutOfRangeException(BuffersThrowHelper.ExceptionArgument.length); - } - } - } -} diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs index 1aba7faf4a01..fb34300a4bd4 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs @@ -528,11 +528,9 @@ private void ParseChunkedPrefix(ReadOnlySequence buffer, out SequencePosit { consumed = buffer.Start; examined = buffer.Start; - var reader = new BufferReader(buffer); - var ch1 = reader.Read(); - var ch2 = reader.Read(); + var reader = new SequenceReader(buffer); - if (ch1 == -1 || ch2 == -1) + if (!reader.TryRead(out var ch1) || !reader.TryRead(out var ch2)) { examined = reader.Position; return; @@ -541,21 +539,20 @@ private void ParseChunkedPrefix(ReadOnlySequence buffer, out SequencePosit var chunkSize = CalculateChunkSize(ch1, 0); ch1 = ch2; - while (reader.ConsumedBytes < MaxChunkPrefixBytes) + while (reader.Consumed < MaxChunkPrefixBytes) { if (ch1 == ';') { consumed = reader.Position; examined = reader.Position; - AddAndCheckConsumedBytes(reader.ConsumedBytes); + AddAndCheckConsumedBytes(reader.Consumed); _inputLength = chunkSize; _mode = Mode.Extension; return; } - ch2 = reader.Read(); - if (ch2 == -1) + if (!reader.TryRead(out ch2)) { examined = reader.Position; return; @@ -566,7 +563,7 @@ private void ParseChunkedPrefix(ReadOnlySequence buffer, out SequencePosit consumed = reader.Position; examined = reader.Position; - AddAndCheckConsumedBytes(reader.ConsumedBytes); + AddAndCheckConsumedBytes(reader.Consumed); _inputLength = chunkSize; _mode = chunkSize > 0 ? Mode.Data : Mode.Trailer; return; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs index 5400a07e17e3..cab563752971 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs @@ -182,25 +182,26 @@ public unsafe bool ParseHeaders(TRequestHandler handler, in ReadOnlySequence(buffer); + var start = default(SequenceReader); var done = false; try { while (!reader.End) { - var span = reader.CurrentSegment; - var remaining = span.Length - reader.CurrentSegmentIndex; + var span = reader.CurrentSpan; + var remaining = span.Length - reader.CurrentSpanIndex; fixed (byte* pBuffer = span) { while (remaining > 0) { - var index = reader.CurrentSegmentIndex; - int ch1; - int ch2; + var index = reader.CurrentSpanIndex; + byte ch1; + byte ch2; var readAhead = false; + bool readSecond = true; // Fast path, we're still looking at the same span if (remaining >= 2) @@ -215,8 +216,8 @@ public unsafe bool ParseHeaders(TRequestHandler handler, in ReadOnlySequence(pBuffer + index, remaining).IndexOf(ByteLF); @@ -308,7 +309,7 @@ public unsafe bool ParseHeaders(TRequestHandler handler, in ReadOnlySequence(first, 0, last, 0)); - reader.Read(); - reader.Read(); - reader.Read(); - Assert.Same(last, reader.Position.GetObject()); - Assert.Equal(0, reader.Position.GetInteger()); - Assert.True(reader.End); - } - - [Fact] - public void PeekReturnsMinusOneByteInTheEnd() - { - var reader = new BufferReader(Factory.CreateWithContent(new byte[] { 1, 2 })); - Assert.Equal(1, reader.Read()); - Assert.Equal(2, reader.Read()); - Assert.Equal(-1, reader.Peek()); - } - - [Fact] - public void AdvanceToEndThenPeekReturnsMinusOne() - { - var reader = new BufferReader(Factory.CreateWithContent(new byte[] { 1, 2, 3, 4, 5 })); - reader.Advance(5); - Assert.True(reader.End); - Assert.Equal(-1, reader.Peek()); - } - - [Fact] - public void AdvancingPastLengthThrows() - { - var reader = new BufferReader(Factory.CreateWithContent(new byte[] { 1, 2, 3, 4, 5 })); - try - { - reader.Advance(6); - Assert.True(false); - } - catch (Exception ex) - { - Assert.True(ex is ArgumentOutOfRangeException); - } - } - - [Fact] - public void CtorFindsFirstNonEmptySegment() - { - var buffer = Factory.CreateWithContent(new byte[] { 1 }); - var reader = new BufferReader(buffer); - - Assert.Equal(1, reader.Peek()); - } - - [Fact] - public void EmptySegmentsAreSkippedOnMoveNext() - { - var buffer = Factory.CreateWithContent(new byte[] { 1, 2 }); - var reader = new BufferReader(buffer); - - Assert.Equal(1, reader.Peek()); - reader.Advance(1); - Assert.Equal(2, reader.Peek()); - } - - [Fact] - public void PeekGoesToEndIfAllEmptySegments() - { - var buffer = Factory.CreateOfSize(0); - var reader = new BufferReader(buffer); - - Assert.Equal(-1, reader.Peek()); - Assert.True(reader.End); - } - - [Fact] - public void AdvanceTraversesSegments() - { - var buffer = Factory.CreateWithContent(new byte[] { 1, 2, 3 }); - var reader = new BufferReader(buffer); - - reader.Advance(2); - Assert.Equal(3, reader.CurrentSegment[reader.CurrentSegmentIndex]); - Assert.Equal(3, reader.Read()); - } - - [Fact] - public void AdvanceThrowsPastLengthMultipleSegments() - { - var buffer = Factory.CreateWithContent(new byte[] { 1, 2, 3 }); - var reader = new BufferReader(buffer); - - try - { - reader.Advance(4); - Assert.True(false); - } - catch (Exception ex) - { - Assert.True(ex is ArgumentOutOfRangeException); - } - } - - [Fact] - public void TakeTraversesSegments() - { - var buffer = Factory.CreateWithContent(new byte[] { 1, 2, 3 }); - var reader = new BufferReader(buffer); - - Assert.Equal(1, reader.Read()); - Assert.Equal(2, reader.Read()); - Assert.Equal(3, reader.Read()); - Assert.Equal(-1, reader.Read()); - } - - [Fact] - public void PeekTraversesSegments() - { - var buffer = Factory.CreateWithContent(new byte[] { 1, 2 }); - var reader = new BufferReader(buffer); - - Assert.Equal(1, reader.CurrentSegment[reader.CurrentSegmentIndex]); - Assert.Equal(1, reader.Read()); - - Assert.Equal(2, reader.CurrentSegment[reader.CurrentSegmentIndex]); - Assert.Equal(2, reader.Peek()); - Assert.Equal(2, reader.Read()); - Assert.Equal(-1, reader.Peek()); - Assert.Equal(-1, reader.Read()); - } - - [Fact] - public void PeekWorksWithEmptySegments() - { - var buffer = Factory.CreateWithContent(new byte[] { 1 }); - var reader = new BufferReader(buffer); - - Assert.Equal(0, reader.CurrentSegmentIndex); - Assert.Equal(1, reader.CurrentSegment.Length); - Assert.Equal(1, reader.Peek()); - Assert.Equal(1, reader.Read()); - Assert.Equal(-1, reader.Peek()); - Assert.Equal(-1, reader.Read()); - } - - [Fact] - public void WorksWithEmptyBuffer() - { - var reader = new BufferReader(Factory.CreateWithContent(new byte[] { })); - - Assert.Equal(0, reader.CurrentSegmentIndex); - Assert.Equal(0, reader.CurrentSegment.Length); - Assert.Equal(-1, reader.Peek()); - Assert.Equal(-1, reader.Read()); - } - - [Theory] - [InlineData(0, false)] - [InlineData(5, false)] - [InlineData(10, false)] - [InlineData(11, true)] - [InlineData(12, true)] - [InlineData(15, true)] - public void ReturnsCorrectCursor(int takes, bool end) - { - var readableBuffer = Factory.CreateWithContent(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }); - var reader = new BufferReader(readableBuffer); - for (int i = 0; i < takes; i++) - { - reader.Read(); - } - - var expected = end ? new byte[] { } : readableBuffer.Slice((long)takes).ToArray(); - Assert.Equal(expected, readableBuffer.Slice(reader.Position).ToArray()); - } - - [Fact] - public void SlicingBufferReturnsCorrectCursor() - { - var buffer = Factory.CreateWithContent(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }); - var sliced = buffer.Slice(2L); - - var reader = new BufferReader(sliced); - Assert.Equal(sliced.ToArray(), buffer.Slice(reader.Position).ToArray()); - Assert.Equal(2, reader.Peek()); - Assert.Equal(0, reader.CurrentSegmentIndex); - } - - [Fact] - public void ReaderIndexIsCorrect() - { - var buffer = Factory.CreateWithContent(new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }); - var reader = new BufferReader(buffer); - - var counter = 1; - while (!reader.End) - { - var span = reader.CurrentSegment; - for (int i = reader.CurrentSegmentIndex; i < span.Length; i++) - { - Assert.Equal(counter++, reader.CurrentSegment[i]); - } - reader.Advance(span.Length); - } - Assert.Equal(buffer.Length, reader.ConsumedBytes); - } - } - -} From a02ac201830aa2396b3b225eb1910af28567dc63 Mon Sep 17 00:00:00 2001 From: Jeremy Kuhne Date: Thu, 7 Feb 2019 15:52:48 -0800 Subject: [PATCH 2/2] Use var --- src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs index cab563752971..3010aecc6173 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs @@ -201,7 +201,7 @@ public unsafe bool ParseHeaders(TRequestHandler handler, in ReadOnlySequence= 2)