From f179339a79165b40e337d737f80a006407bf88fc Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 12 Jul 2018 11:58:49 -0700 Subject: [PATCH] Combine BufferWriter and CountingBufferWriter --- .../BenchmarkApplication.cs | 6 +- src/Kestrel.Core/Internal/BufferWriter.cs | 11 ++- src/Kestrel.Core/Internal/Http/ChunkWriter.cs | 4 +- .../Internal/Http/CountingBufferWriter.cs | 99 ------------------- .../Internal/Http/Http1OutputProducer.cs | 4 +- .../Internal/Http/HttpHeaders.Generated.cs | 2 +- .../Internal/Http/HttpProtocol.cs | 2 +- .../Internal/Http/HttpResponseHeaders.cs | 2 +- .../Internal/Http/PipelineExtensions.cs | 8 +- test/Kestrel.Core.Tests/BufferWriterTests.cs | 58 ++++------- .../PipelineExtensionTests.cs | 12 +-- tools/CodeGenerator/KnownHeaders.cs | 2 +- 12 files changed, 46 insertions(+), 164 deletions(-) delete mode 100644 src/Kestrel.Core/Internal/Http/CountingBufferWriter.cs diff --git a/benchmarkapps/PlatformBenchmarks/BenchmarkApplication.cs b/benchmarkapps/PlatformBenchmarks/BenchmarkApplication.cs index d65452c13c..6551bee358 100644 --- a/benchmarkapps/PlatformBenchmarks/BenchmarkApplication.cs +++ b/benchmarkapps/PlatformBenchmarks/BenchmarkApplication.cs @@ -78,7 +78,7 @@ namespace PlatformBenchmarks } private static void PlainText(PipeWriter pipeWriter) { - var writer = new CountingBufferWriter(pipeWriter); + var writer = new BufferWriter(pipeWriter); // HTTP 1.1 OK writer.Write(_http11OK); @@ -105,7 +105,7 @@ namespace PlatformBenchmarks private static void Json(PipeWriter pipeWriter) { - var writer = new CountingBufferWriter(pipeWriter); + var writer = new BufferWriter(pipeWriter); // HTTP 1.1 OK writer.Write(_http11OK); @@ -134,7 +134,7 @@ namespace PlatformBenchmarks private static void Default(PipeWriter pipeWriter) { - var writer = new CountingBufferWriter(pipeWriter); + var writer = new BufferWriter(pipeWriter); // HTTP 1.1 OK writer.Write(_http11OK); diff --git a/src/Kestrel.Core/Internal/BufferWriter.cs b/src/Kestrel.Core/Internal/BufferWriter.cs index 2b63ea920c..1f33f3e4cb 100644 --- a/src/Kestrel.Core/Internal/BufferWriter.cs +++ b/src/Kestrel.Core/Internal/BufferWriter.cs @@ -1,25 +1,27 @@ -// 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. +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Runtime.CompilerServices; namespace System.Buffers { - internal ref struct BufferWriter where T: IBufferWriter + internal ref struct BufferWriter where T : IBufferWriter { private T _output; private Span _span; private int _buffered; + private long _bytesCommitted; public BufferWriter(T output) { _buffered = 0; + _bytesCommitted = 0; _output = output; _span = output.GetSpan(); } public Span Span => _span; + public long BytesCommitted => _bytesCommitted; [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Commit() @@ -27,6 +29,7 @@ namespace System.Buffers var buffered = _buffered; if (buffered > 0) { + _bytesCommitted += buffered; _buffered = 0; _output.Advance(buffered); } diff --git a/src/Kestrel.Core/Internal/Http/ChunkWriter.cs b/src/Kestrel.Core/Internal/Http/ChunkWriter.cs index 2184937b07..3d8cc4566b 100644 --- a/src/Kestrel.Core/Internal/Http/ChunkWriter.cs +++ b/src/Kestrel.Core/Internal/Http/ChunkWriter.cs @@ -48,14 +48,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return new ArraySegment(bytes, offset, 10 - offset); } - internal static int WriteBeginChunkBytes(ref CountingBufferWriter start, int dataCount) + internal static int WriteBeginChunkBytes(ref BufferWriter start, int dataCount) { var chunkSegment = BeginChunkBytes(dataCount); start.Write(new ReadOnlySpan(chunkSegment.Array, chunkSegment.Offset, chunkSegment.Count)); return chunkSegment.Count; } - internal static void WriteEndChunkBytes(ref CountingBufferWriter start) + internal static void WriteEndChunkBytes(ref BufferWriter start) { start.Write(new ReadOnlySpan(_endChunkBytes.Array, _endChunkBytes.Offset, _endChunkBytes.Count)); } diff --git a/src/Kestrel.Core/Internal/Http/CountingBufferWriter.cs b/src/Kestrel.Core/Internal/Http/CountingBufferWriter.cs deleted file mode 100644 index e3299c6027..0000000000 --- a/src/Kestrel.Core/Internal/Http/CountingBufferWriter.cs +++ /dev/null @@ -1,99 +0,0 @@ - -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Runtime.CompilerServices; - -namespace System.Buffers -{ - // TODO: Once this is public, update the actual CountingBufferWriter in the Common repo, - // and go back to using that. - internal ref struct CountingBufferWriter where T: IBufferWriter - { - private T _output; - private Span _span; - private int _buffered; - private long _bytesCommitted; - - public CountingBufferWriter(T output) - { - _buffered = 0; - _bytesCommitted = 0; - _output = output; - _span = output.GetSpan(); - } - - public Span Span => _span; - public long BytesCommitted => _bytesCommitted; - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Commit() - { - var buffered = _buffered; - if (buffered > 0) - { - _bytesCommitted += buffered; - _buffered = 0; - _output.Advance(buffered); - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Advance(int count) - { - _buffered += count; - _span = _span.Slice(count); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Write(ReadOnlySpan source) - { - if (_span.Length >= source.Length) - { - source.CopyTo(_span); - Advance(source.Length); - } - else - { - WriteMultiBuffer(source); - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Ensure(int count = 1) - { - if (_span.Length < count) - { - EnsureMore(count); - } - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private void EnsureMore(int count = 0) - { - if (_buffered > 0) - { - Commit(); - } - - _output.GetMemory(count); - _span = _output.GetSpan(); - } - - private void WriteMultiBuffer(ReadOnlySpan source) - { - while (source.Length > 0) - { - if (_span.Length == 0) - { - EnsureMore(); - } - - var writable = Math.Min(source.Length, _span.Length); - source.Slice(0, writable).CopyTo(_span); - source = source.Slice(writable); - Advance(writable); - } - } - } -} diff --git a/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs b/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs index 0fd62301bd..685980d1c6 100644 --- a/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs @@ -128,7 +128,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } var buffer = _pipeWriter; - var writer = new CountingBufferWriter(buffer); + var writer = new BufferWriter(buffer); writer.Write(_bytesHttpVersion11); var statusBytes = ReasonPhrases.ToStatusBytes(statusCode, reasonPhrase); @@ -210,7 +210,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } writableBuffer = _pipeWriter; - var writer = new CountingBufferWriter(writableBuffer); + var writer = new BufferWriter(writableBuffer); if (buffer.Length > 0) { writer.Write(buffer); diff --git a/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs b/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs index f81e87a06c..d84f15706d 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.Generated.cs @@ -7765,7 +7765,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return true; } - internal void CopyToFast(ref CountingBufferWriter output) + internal void CopyToFast(ref BufferWriter output) { var tempBits = _bits | (_contentLength.HasValue ? -9223372036854775808L : 0); diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index 0c759c4451..b0ae93147d 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -936,7 +936,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var bytesWritten = 0L; if (buffer.Length > 0) { - var writer = new CountingBufferWriter(writableBuffer); + var writer = new BufferWriter(writableBuffer); ChunkWriter.WriteBeginChunkBytes(ref writer, buffer.Length); writer.Write(buffer.Span); diff --git a/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs b/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs index a4b81cf69a..1df80f3dc6 100644 --- a/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpResponseHeaders.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return GetEnumerator(); } - internal void CopyTo(ref CountingBufferWriter buffer) + internal void CopyTo(ref BufferWriter buffer) { CopyToFast(ref buffer); if (MaybeUnknown != null) diff --git a/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs b/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs index fce822b6c5..e56c43b23c 100644 --- a/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs +++ b/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs @@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return result; } - internal static unsafe void WriteAsciiNoValidation(ref this CountingBufferWriter buffer, string data) + internal static unsafe void WriteAsciiNoValidation(ref this BufferWriter buffer, string data) { if (string.IsNullOrEmpty(data)) { @@ -69,7 +69,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static unsafe void WriteNumeric(ref this CountingBufferWriter buffer, ulong number) + internal static unsafe void WriteNumeric(ref this BufferWriter buffer, ulong number) { const byte AsciiDigitStart = (byte)'0'; @@ -119,7 +119,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } [MethodImpl(MethodImplOptions.NoInlining)] - private static void WriteNumericMultiWrite(ref this CountingBufferWriter buffer, ulong number) + private static void WriteNumericMultiWrite(ref this BufferWriter buffer, ulong number) { const byte AsciiDigitStart = (byte)'0'; @@ -140,7 +140,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } [MethodImpl(MethodImplOptions.NoInlining)] - private unsafe static void WriteAsciiMultiWrite(ref this CountingBufferWriter buffer, string data) + private unsafe static void WriteAsciiMultiWrite(ref this BufferWriter buffer, string data) { var remaining = data.Length; diff --git a/test/Kestrel.Core.Tests/BufferWriterTests.cs b/test/Kestrel.Core.Tests/BufferWriterTests.cs index 8a071385a4..2060ccdb32 100644 --- a/test/Kestrel.Core.Tests/BufferWriterTests.cs +++ b/test/Kestrel.Core.Tests/BufferWriterTests.cs @@ -4,7 +4,6 @@ using System.Buffers; using System.Collections.Generic; using System.Linq; -using System.Threading.Tasks; using Xunit; namespace System.IO.Pipelines.Tests @@ -73,15 +72,20 @@ namespace System.IO.Pipelines.Tests [InlineData(1, 2)] [InlineData(2, 1)] [InlineData(1, 1)] - public void CanWriteWithOffsetAndLenght(int offset, int length) + public void CanWriteWithOffsetAndLength(int offset, int length) { BufferWriter writer = new BufferWriter(Pipe.Writer); var array = new byte[] { 1, 2, 3 }; writer.Write(new Span(array, offset, length)); + + Assert.Equal(0, writer.BytesCommitted); + writer.Commit(); + Assert.Equal(length, writer.BytesCommitted); Assert.Equal(array.Skip(offset).Take(length).ToArray(), Read()); + Assert.Equal(length, writer.BytesCommitted); } [Fact] @@ -94,6 +98,7 @@ namespace System.IO.Pipelines.Tests writer.Write(new Span(array, 0, array.Length)); writer.Commit(); + Assert.Equal(0, writer.BytesCommitted); Assert.Equal(array, Read()); } @@ -105,6 +110,7 @@ namespace System.IO.Pipelines.Tests writer.Write(new byte[] { 1, 2, 3 }); writer.Commit(); + Assert.Equal(3, writer.BytesCommitted); Assert.Equal(new byte[] { 1, 2, 3 }, Read()); } @@ -118,6 +124,7 @@ namespace System.IO.Pipelines.Tests writer.Write(new byte[] { 3 }); writer.Commit(); + Assert.Equal(3, writer.BytesCommitted); Assert.Equal(new byte[] { 1, 2, 3 }, Read()); } @@ -133,6 +140,7 @@ namespace System.IO.Pipelines.Tests writer.Write(expectedBytes); writer.Commit(); + Assert.Equal(expectedBytes.LongLength, writer.BytesCommitted); Assert.Equal(expectedBytes, Read()); } @@ -142,6 +150,7 @@ namespace System.IO.Pipelines.Tests BufferWriter writer = new BufferWriter(Pipe.Writer); writer.Ensure(10); Assert.True(writer.Span.Length > 10); + Assert.Equal(0, writer.BytesCommitted); Assert.Equal(new byte[] { }, Read()); } @@ -164,6 +173,7 @@ namespace System.IO.Pipelines.Tests writer.Write(new byte[] { 1, 2, 3 }); writer.Commit(); + Assert.Equal(3, writer.BytesCommitted); Assert.Equal(initialLength - 3, writer.Span.Length); Assert.Equal(Pipe.Writer.GetMemory().Length, writer.Span.Length); Assert.Equal(new byte[] { 1, 2, 3 }, Read()); @@ -175,49 +185,17 @@ namespace System.IO.Pipelines.Tests [InlineData(500)] [InlineData(5000)] [InlineData(50000)] - public async Task WriteLargeDataBinary(int length) + public void WriteLargeDataBinary(int length) { var data = new byte[length]; new Random(length).NextBytes(data); - PipeWriter output = Pipe.Writer; - output.Write(data); - await output.FlushAsync(); - ReadResult result = await Pipe.Reader.ReadAsync(); - ReadOnlySequence input = result.Buffer; - Assert.Equal(data, input.ToArray()); - Pipe.Reader.AdvanceTo(input.End); - } + BufferWriter writer = new BufferWriter(Pipe.Writer); + writer.Write(data); + writer.Commit(); - [Fact] - public async Task CanWriteNothingToBuffer() - { - PipeWriter buffer = Pipe.Writer; - buffer.GetMemory(0); - buffer.Advance(0); // doing nothing, the hard way - await buffer.FlushAsync(); - } - - [Fact] - public void EmptyWriteDoesNotThrow() - { - Pipe.Writer.Write(new byte[0]); - } - - [Fact] - public void ThrowsOnAdvanceOverMemorySize() - { - Memory buffer = Pipe.Writer.GetMemory(1); - var exception = Assert.Throws(() => Pipe.Writer.Advance(buffer.Length + 1)); - Assert.Equal("Can't advance past buffer size.", exception.Message); - } - - [Fact] - public void ThrowsOnAdvanceWithNoMemory() - { - PipeWriter buffer = Pipe.Writer; - var exception = Assert.Throws(() => buffer.Advance(1)); - Assert.Equal("No writing operation. Make sure GetMemory() was called.", exception.Message); + Assert.Equal(length, writer.BytesCommitted); + Assert.Equal(data, Read()); } } } diff --git a/test/Kestrel.Core.Tests/PipelineExtensionTests.cs b/test/Kestrel.Core.Tests/PipelineExtensionTests.cs index 43c385772e..e3a89832da 100644 --- a/test/Kestrel.Core.Tests/PipelineExtensionTests.cs +++ b/test/Kestrel.Core.Tests/PipelineExtensionTests.cs @@ -36,7 +36,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public void WritesNumericToAscii(ulong number) { var writerBuffer = _pipe.Writer; - var writer = new CountingBufferWriter(writerBuffer); + var writer = new BufferWriter(writerBuffer); writer.WriteNumeric(number); writer.Commit(); writerBuffer.FlushAsync().GetAwaiter().GetResult(); @@ -54,7 +54,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public void WritesNumericAcrossSpanBoundaries(int gapSize) { var writerBuffer = _pipe.Writer; - var writer = new CountingBufferWriter(writerBuffer); + var writer = new BufferWriter(writerBuffer); // almost fill up the first block var spacer = new byte[writer.Span.Length - gapSize]; writer.Write(spacer); @@ -85,7 +85,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public void EncodesAsAscii(string input, byte[] expected) { var pipeWriter = _pipe.Writer; - var writer = new CountingBufferWriter(pipeWriter); + var writer = new BufferWriter(pipeWriter); writer.WriteAsciiNoValidation(input); writer.Commit(); pipeWriter.FlushAsync().GetAwaiter().GetResult(); @@ -115,7 +115,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // WriteAscii doesn't validate if characters are in the ASCII range // but it shouldn't produce more than one byte per character var writerBuffer = _pipe.Writer; - var writer = new CountingBufferWriter(writerBuffer); + var writer = new BufferWriter(writerBuffer); writer.WriteAsciiNoValidation(input); writer.Commit(); writerBuffer.FlushAsync().GetAwaiter().GetResult(); @@ -129,7 +129,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { const byte maxAscii = 0x7f; var writerBuffer = _pipe.Writer; - var writer = new CountingBufferWriter(writerBuffer); + var writer = new BufferWriter(writerBuffer); for (var i = 0; i < maxAscii; i++) { writer.WriteAsciiNoValidation(new string((char)i, 1)); @@ -159,7 +159,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { var testString = new string(' ', stringLength); var writerBuffer = _pipe.Writer; - var writer = new CountingBufferWriter(writerBuffer); + var writer = new BufferWriter(writerBuffer); // almost fill up the first block var spacer = new byte[writer.Span.Length - gapSize]; writer.Write(spacer); diff --git a/tools/CodeGenerator/KnownHeaders.cs b/tools/CodeGenerator/KnownHeaders.cs index badf9087a7..08646dc61a 100644 --- a/tools/CodeGenerator/KnownHeaders.cs +++ b/tools/CodeGenerator/KnownHeaders.cs @@ -548,7 +548,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return true; }} {(loop.ClassName == "HttpResponseHeaders" ? $@" - internal void CopyToFast(ref CountingBufferWriter output) + internal void CopyToFast(ref BufferWriter output) {{ var tempBits = _bits | (_contentLength.HasValue ? {1L << 63}L : 0); {Each(loop.Headers.Where(header => header.Identifier != "ContentLength").OrderBy(h => !h.PrimaryHeader), header => $@"