From 434da683fc2254df76cdf63df190a3b9cdb05c4a Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 12 Jan 2016 12:39:38 -0800 Subject: [PATCH] Improve buffering of Razor output in MVC These changes are aimed at significantly improving the performance of MVC/Razor when a large amount of content is in play or a large number of TagHelpers are used. A few issues addressed: - Buffer sync writes after a flush has occurred so that we can write them asyncronously. The issue is that an IHtmlContent can only do sync writes. This is very bad for Kestrel in general. Doing these writes async is better for our overall perf, and the buffer that we use for it is from the pool. - 'Flatten' ViewBuffers when possible. A page with lots of TagHelpers can end up renting a ViewBuffer and only write 2-3 things into it. When a ViewBuffer sees another ViewBuffer we can either steal its pages, or copy data out and 'return' its pages. This lets us use 3-4 buffers for a large Razor page instead of hundreds. --- .../RazorTextWriter.cs | 12 +- .../RazorView.cs | 6 +- .../Internal/IViewBufferScope.cs | 16 ++ .../Internal/MemoryPoolViewBufferScope.cs | 61 ++++- .../Internal/ViewBuffer.cs | 111 +++++--- .../Internal/ViewBufferPage.cs | 23 ++ .../Internal/ViewBufferTextWriter.cs | 211 +++++++++++++++ .../Internal/ViewBufferValue.cs | 26 ++ .../RazorTextWriterTest.cs | 9 +- .../Internal/TestViewBufferScope.cs | 16 ++ .../Internal/ViewBufferTest.cs | 252 ++++++++++++++++-- .../ViewBufferTextWriterTest.cs | 176 ++++++++++++ 12 files changed, 841 insertions(+), 78 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferPage.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferTextWriter.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewBufferTextWriterTest.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorTextWriter.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorTextWriter.cs index 6c204a37e5..e82f690c6e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorTextWriter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorTextWriter.cs @@ -98,15 +98,9 @@ namespace Microsoft.AspNetCore.Mvc.Razor /// public override void Write(IHtmlContent value) { - var htmlTextWriter = TargetWriter as HtmlTextWriter; - if (htmlTextWriter == null) - { - value.WriteTo(TargetWriter, HtmlEncoder); - } - else - { - htmlTextWriter.Write(value); - } + // Perf: We don't special case 'TargetWriter is HtmlTextWriter' here, because want to delegate to the + // IHtmlContent if it wants to 'flatten' itself or not. This is an important optimization for TagHelpers. + value.WriteTo(TargetWriter, HtmlEncoder); } /// diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorView.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorView.cs index b35c256a4d..d823f62370 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorView.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorView.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Buffers; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -233,7 +234,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor if (bodyWriter.IsBuffering) { // Only copy buffered content to the Output if we're currently buffering. - await bodyWriter.Buffer.WriteToAsync(context.Writer, _htmlEncoder); + using (var writer = _bufferScope.CreateWriter(context.Writer)) + { + await bodyWriter.Buffer.WriteToAsync(writer, _htmlEncoder); + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/IViewBufferScope.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/IViewBufferScope.cs index 9b46ce27cc..6d90277737 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/IViewBufferScope.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/IViewBufferScope.cs @@ -1,6 +1,8 @@ // 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.IO; + namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { /// @@ -13,5 +15,19 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal /// /// The . ViewBufferValue[] GetSegment(); + + /// + /// Returns a that can be reused. + /// + /// The . + void ReturnSegment(ViewBufferValue[] segment); + + /// + /// Creates a that will delegate to the provided + /// . + /// + /// The . + /// A . + ViewBufferTextWriter CreateWriter(TextWriter writer); } } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/MemoryPoolViewBufferScope.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/MemoryPoolViewBufferScope.cs index 02a11e7695..362f5ab72e 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/MemoryPoolViewBufferScope.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/MemoryPoolViewBufferScope.cs @@ -4,6 +4,7 @@ using System; using System.Buffers; using System.Collections.Generic; +using System.IO; namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { @@ -13,18 +14,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal public class MemoryPoolViewBufferScope : IViewBufferScope, IDisposable { public static readonly int SegmentSize = 512; - private readonly ArrayPool _pool; + private readonly ArrayPool _viewBufferPool; + private readonly ArrayPool _charPool; + private List _available; private List _leased; private bool _disposed; /// /// Initializes a new instance of . /// - /// The for creating - /// instances. - public MemoryPoolViewBufferScope(ArrayPool pool) + /// + /// The for creating instances. + /// + /// + /// The for creating instances. + /// + public MemoryPoolViewBufferScope(ArrayPool viewBufferPool, ArrayPool charPool) { - _pool = pool; + _viewBufferPool = viewBufferPool; + _charPool = charPool; } /// @@ -42,20 +50,57 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal ViewBufferValue[] segment = null; + // Reuse pages that have been returned before going back to the memory pool. + if (_available != null && _available.Count > 0) + { + segment = _available[_available.Count - 1]; + _available.RemoveAt(_available.Count - 1); + return segment; + } + try { - segment = _pool.Rent(SegmentSize); + segment = _viewBufferPool.Rent(SegmentSize); _leased.Add(segment); } catch when (segment != null) { - _pool.Return(segment); + _viewBufferPool.Return(segment); throw; } return segment; } + /// + public void ReturnSegment(ViewBufferValue[] segment) + { + if (segment == null) + { + throw new ArgumentNullException(nameof(segment)); + } + + Array.Clear(segment, 0, segment.Length); + + if (_available == null) + { + _available = new List(); + } + + _available.Add(segment); + } + + /// + public ViewBufferTextWriter CreateWriter(TextWriter writer) + { + if (writer == null) + { + throw new ArgumentNullException(nameof(writer)); + } + + return new ViewBufferTextWriter(_charPool, writer); + } + /// public void Dispose() { @@ -70,7 +115,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal for (var i = 0; i < _leased.Count; i++) { - _pool.Return(_leased[i]); + _viewBufferPool.Return(_leased[i]); } _leased.Clear(); diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBuffer.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBuffer.cs index 137e9e8ada..b6fcb0ed1f 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBuffer.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBuffer.cs @@ -2,9 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Buffers; using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Text; using System.Text.Encodings.Web; using System.Threading.Tasks; using Microsoft.AspNetCore.Html; @@ -35,17 +37,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal _bufferScope = bufferScope; _name = name; + + Pages = new List(); } /// /// Gets the backing buffer. /// - public IList BufferSegments { get; private set; } - - /// - /// Gets the count of entries in the last element of . - /// - public int CurrentCount { get; private set; } + public IList Pages { get; } /// public IHtmlContentBuilder Append(string unencoded) @@ -67,7 +66,48 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return this; } - AppendValue(new ViewBufferValue(content)); + // Perf: special case ViewBuffers so we can 'combine' them. + var otherBuffer = content as ViewBuffer; + if (otherBuffer == null) + { + AppendValue(new ViewBufferValue(content)); + return this; + } + + for (var i = 0; i < otherBuffer.Pages.Count; i++) + { + var otherPage = otherBuffer.Pages[i]; + var currentPage = Pages.Count == 0 ? null : Pages[Pages.Count - 1]; + + // If the other page is less or equal to than half full, let's copy it's to the current page if + // possible. + var isLessThanHalfFull = 2 * otherPage.Count <= otherPage.Capacity; + if (isLessThanHalfFull && + currentPage != null && + currentPage.Capacity - currentPage.Count >= otherPage.Count) + { + // We have room, let's copy the items. + Array.Copy( + sourceArray: otherPage.Buffer, + sourceIndex: 0, + destinationArray: currentPage.Buffer, + destinationIndex: currentPage.Count, + length: otherPage.Count); + + currentPage.Count += otherPage.Count; + + // Now we can return this page, and it can be reused in the scope of this request. + _bufferScope.ReturnSegment(otherPage.Buffer); + } + else + { + // Otherwise, let's just take the the page from the other buffer. + Pages.Add(otherPage); + } + + } + + otherBuffer.Clear(); return this; } @@ -86,35 +126,37 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal private void AppendValue(ViewBufferValue value) { - ViewBufferValue[] segment; - if (BufferSegments == null) + var page = GetCurrentPage(); + page.Append(value); + } + + private ViewBufferPage GetCurrentPage() + { + ViewBufferPage page; + if (Pages.Count == 0) { - BufferSegments = new List(1); - segment = _bufferScope.GetSegment(); - BufferSegments.Add(segment); + page = new ViewBufferPage(_bufferScope.GetSegment()); + Pages.Add(page); } else { - segment = BufferSegments[BufferSegments.Count - 1]; - if (CurrentCount == segment.Length) + page = Pages[Pages.Count - 1]; + if (page.IsFull) { - segment = _bufferScope.GetSegment(); - BufferSegments.Add(segment); - CurrentCount = 0; + page = new ViewBufferPage(_bufferScope.GetSegment()); + Pages.Add(page); } } - segment[CurrentCount] = value; - CurrentCount++; + return page; } /// public IHtmlContentBuilder Clear() { - if (BufferSegments != null) + if (Pages != null) { - CurrentCount = 0; - BufferSegments = null; + Pages.Clear(); } return this; @@ -123,7 +165,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal /// public void WriteTo(TextWriter writer, HtmlEncoder encoder) { - if (BufferSegments == null) + if (Pages == null) { return; } @@ -135,14 +177,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return; } - for (var i = 0; i < BufferSegments.Count; i++) + for (var i = 0; i < Pages.Count; i++) { - var segment = BufferSegments[i]; - var count = i == BufferSegments.Count - 1 ? CurrentCount : segment.Length; - - for (var j = 0; j < count; j++) + var page = Pages[i]; + for (var j = 0; j < page.Count; j++) { - var value = segment[j]; + var value = page.Buffer[j]; var valueAsString = value.Value as string; if (valueAsString != null) @@ -169,7 +209,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal /// A which will complete once content has been written. public async Task WriteToAsync(TextWriter writer, HtmlEncoder encoder) { - if (BufferSegments == null) + if (Pages == null) { return; } @@ -181,14 +221,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return; } - for (var i = 0; i < BufferSegments.Count; i++) + for (var i = 0; i < Pages.Count; i++) { - var segment = BufferSegments[i]; - var count = i == BufferSegments.Count - 1 ? CurrentCount : segment.Length; - - for (var j = 0; j < count; j++) + var page = Pages[i]; + for (var j = 0; j < page.Count; j++) { - var value = segment[j]; + var value = page.Buffer[j]; var valueAsString = value.Value as string; if (valueAsString != null) @@ -208,6 +246,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal if (valueAsHtmlContent != null) { valueAsHtmlContent.WriteTo(writer, encoder); + await writer.FlushAsync(); continue; } } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferPage.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferPage.cs new file mode 100644 index 0000000000..b561e71825 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferPage.cs @@ -0,0 +1,23 @@ +// 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. + +namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal +{ + public class ViewBufferPage + { + public ViewBufferPage(ViewBufferValue[] buffer) + { + Buffer = buffer; + } + + public ViewBufferValue[] Buffer { get; } + + public int Capacity => Buffer.Length; + + public int Count { get; set; } + + public bool IsFull => Count == Capacity; + + public void Append(ViewBufferValue value) => Buffer[Count++] = value; + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferTextWriter.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferTextWriter.cs new file mode 100644 index 0000000000..43110e459d --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferTextWriter.cs @@ -0,0 +1,211 @@ +// 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; +using System.Buffers; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal +{ + public class ViewBufferTextWriter : TextWriter + { + public const int PageSize = 1024; + + private readonly TextWriter _inner; + private readonly List _pages; + private readonly ArrayPool _pool; + + private int _currentPage; + private int _currentIndex; // The next 'free' character + + public ViewBufferTextWriter(ArrayPool pool, TextWriter inner) + { + _pool = pool; + _inner = inner; + _pages = new List(); + } + + public override Encoding Encoding => _inner.Encoding; + + public override void Flush() + { + // Don't do anything. We'll call FlushAsync. + } + + public override async Task FlushAsync() + { + if (_pages.Count == 0) + { + return; + } + + for (var i = 0; i <= _currentPage; i++) + { + var page = _pages[i]; + + var count = i == _currentPage ? _currentIndex : page.Length; + if (count > 0) + { + await _inner.WriteAsync(page, 0, count); + } + } + + // Return all but one of the pages. This way if someone writes a large chunk of + // content, we can return those buffers and avoid holding them for the whole + // page's lifetime. + for (var i = _pages.Count - 1; i > 0; i--) + { + var page = _pages[i]; + + try + { + _pages.RemoveAt(i); + } + finally + { + _pool.Return(page); + } + } + + _currentPage = 0; + _currentIndex = 0; + } + + public override void Write(char value) + { + var page = GetCurrentPage(); + page[_currentIndex++] = value; + } + + public override void Write(char[] buffer) + { + Write(buffer, 0, buffer.Length); + } + + public override void Write(char[] buffer, int index, int count) + { + while (count > 0) + { + var page = GetCurrentPage(); + var copyLength = Math.Min(count, page.Length - _currentIndex); + Debug.Assert(copyLength > 0); + + Array.Copy( + buffer, + index, + page, + _currentIndex, + copyLength); + + _currentIndex += copyLength; + index += copyLength; + + count -= copyLength; + } + } + + public override void Write(string value) + { + var index = 0; + var count = value.Length; + + while (count > 0) + { + var page = GetCurrentPage(); + var copyLength = Math.Min(count, page.Length - _currentIndex); + Debug.Assert(copyLength > 0); + + value.CopyTo( + index, + page, + _currentIndex, + copyLength); + + _currentIndex += copyLength; + index += copyLength; + + count -= copyLength; + } + } + + public override Task WriteAsync(char value) + { + return _inner.WriteAsync(value); + } + + public override Task WriteAsync(char[] buffer, int index, int count) + { + return _inner.WriteAsync(buffer, index, count); + } + + public override Task WriteAsync(string value) + { + return _inner.WriteAsync(value); + } + + private char[] GetCurrentPage() + { + char[] page = null; + if (_pages.Count == 0) + { + Debug.Assert(_currentPage == 0); + Debug.Assert(_currentIndex == 0); + + try + { + page = _pool.Rent(PageSize); + _pages.Add(page); + } + catch when (page != null) + { + _pool.Return(page); + throw; + } + + return page; + } + + Debug.Assert(_pages.Count > _currentPage); + page = _pages[_currentPage]; + + if (_currentIndex == page.Length) + { + // Current page is full. + _currentPage++; + _currentIndex = 0; + + if (_pages.Count == _currentPage) + { + try + { + page = _pool.Rent(PageSize); + _pages.Add(page); + } + catch when (page != null) + { + _pool.Return(page); + throw; + } + } + } + + return page; + } + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + + for (var i = 0; i < _pages.Count; i++) + { + _pool.Return(_pages[i]); + } + + _pages.Clear(); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferValue.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferValue.cs index 28f4e61e4a..12b32e8caf 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferValue.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBufferValue.cs @@ -1,6 +1,9 @@ // 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.Diagnostics; +using System.IO; +using System.Text.Encodings.Web; using Microsoft.AspNetCore.Html; namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal @@ -8,6 +11,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal /// /// Encapsulates a string or value. /// + [DebuggerDisplay("{DebuggerToString()}")] public struct ViewBufferValue { /// @@ -32,5 +36,27 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal /// Gets the value. /// public object Value { get; } + + private string DebuggerToString() + { + using (var writer = new StringWriter()) + { + var valueAsString = Value as string; + if (valueAsString != null) + { + writer.Write(valueAsString); + return writer.ToString(); + } + + var valueAsContent = Value as IHtmlContent; + if (valueAsContent != null) + { + valueAsContent.WriteTo(writer, HtmlEncoder.Default); + return writer.ToString(); + } + + return "(null)"; + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorTextWriterTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorTextWriterTest.cs index 2e87bbc503..ecb384a9f2 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorTextWriterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorTextWriterTest.cs @@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test writer.Write(2.718m); // Assert - Assert.Null(buffer.BufferSegments); + Assert.Null(buffer.Pages); foreach (var item in expected) { unbufferedWriter.Verify(v => v.Write(item), Times.Once()); @@ -90,7 +90,6 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test await writer.WriteLineAsync(buffer1); // Assert - Assert.Null(buffer.BufferSegments); unbufferedWriter.Verify(v => v.Write('x'), Times.Once()); unbufferedWriter.Verify(v => v.Write(buffer1, 1, 2), Times.Once()); unbufferedWriter.Verify(v => v.Write(buffer1, 0, 4), Times.Once()); @@ -117,7 +116,6 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test await writer.WriteLineAsync("gh"); // Assert - Assert.Null(buffer.BufferSegments); unbufferedWriter.Verify(v => v.Write("a"), Times.Once()); unbufferedWriter.Verify(v => v.WriteLine("ab"), Times.Once()); unbufferedWriter.Verify(v => v.WriteAsync("ef"), Times.Once()); @@ -160,7 +158,6 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test writer.WriteLine(3L); // Assert - Assert.Null(buffer.BufferSegments); unbufferedWriter.Verify(v => v.Write("False"), Times.Once()); unbufferedWriter.Verify(v => v.Write("1.1"), Times.Once()); unbufferedWriter.Verify(v => v.Write("3"), Times.Once()); @@ -227,8 +224,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test private static object[] GetValues(ViewBuffer buffer) { - return buffer.BufferSegments - .SelectMany(c => c) + return buffer.Pages + .SelectMany(c => c.Buffer) .Select(d => d.Value) .TakeWhile(d => d != null) .ToArray(); diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TestViewBufferScope.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TestViewBufferScope.cs index e437ace6ef..c9163e9d5a 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TestViewBufferScope.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/TestViewBufferScope.cs @@ -1,6 +1,10 @@ // 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.Buffers; +using System.Collections.Generic; +using System.IO; + namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { public class TestViewBufferScope : IViewBufferScope @@ -13,6 +17,18 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal _bufferSize = bufferSize; } + public IList ReturnedBuffers { get; } = new List(); + public ViewBufferValue[] GetSegment() => new ViewBufferValue[_bufferSize]; + + public void ReturnSegment(ViewBufferValue[] segment) + { + ReturnedBuffers.Add(segment); + } + + public ViewBufferTextWriter CreateWriter(TextWriter writer) + { + return new ViewBufferTextWriter(ArrayPool.Shared, writer); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ViewBufferTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ViewBufferTest.cs index af3f0687d6..f46f385b29 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ViewBufferTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ViewBufferTest.cs @@ -1,6 +1,7 @@ // 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.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -24,9 +25,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal buffer.Append("Hello world"); // Assert - var segment = Assert.Single(buffer.BufferSegments); - Assert.Equal(1, buffer.CurrentCount); - Assert.Equal("Hello world", segment[0].Value); + var page = Assert.Single(buffer.Pages); + Assert.Equal(1, page.Count); + Assert.Equal("Hello world", page.Buffer[0].Value); } [Fact] @@ -40,9 +41,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal buffer.AppendHtml(content); // Assert - var segment = Assert.Single(buffer.BufferSegments); - Assert.Equal(1, buffer.CurrentCount); - Assert.Same(content, segment[0].Value); + var page = Assert.Single(buffer.Pages); + Assert.Equal(1, page.Count); + Assert.Same(content, page.Buffer[0].Value); } [Fact] @@ -56,14 +57,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal buffer.AppendHtml(value); // Assert - var segment = Assert.Single(buffer.BufferSegments); - Assert.Equal(1, buffer.CurrentCount); - var htmlString = Assert.IsType(segment[0].Value); + var page = Assert.Single(buffer.Pages); + Assert.Equal(1, page.Count); + var htmlString = Assert.IsType(page.Buffer[0].Value); Assert.Equal("Hello world", htmlString.ToString()); } [Fact] - public void Append_CreatesNewSegments_WhenCurrentSegmentIsFull() + public void Append_CreatesNewPages_WhenCurrentPageIsFull() { // Arrange var buffer = new ViewBuffer(new TestViewBufferScope(), "some-name"); @@ -78,12 +79,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal buffer.Append("world"); // Assert - Assert.Equal(2, buffer.CurrentCount); - Assert.Collection(buffer.BufferSegments, - segment => Assert.Equal(expected, segment.Select(v => v.Value)), - segment => + Assert.Equal(2, buffer.Pages.Count); + Assert.Collection(buffer.Pages, + page => Assert.Equal(expected, page.Buffer.Select(v => v.Value)), + page => { - var array = segment; + var array = page.Buffer; Assert.Equal("Hello", array[0].Value); Assert.Equal("world", array[1].Value); }); @@ -106,9 +107,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal buffer.Append("world"); // Assert - var segment = Assert.Single(buffer.BufferSegments); - Assert.Equal(1, buffer.CurrentCount); - Assert.Equal("world", segment[0].Value); + var page = Assert.Single(buffer.Pages); + Assert.Equal(1, page.Count); + Assert.Equal("world", page.Buffer[0].Value); } [Fact] @@ -224,5 +225,220 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal // Assert Assert.Equal(expected, writer.ToString()); } + + [Fact] + public void AppendHtml_ViewBuffer_TakesPage_IfOriginalIsEmpty() + { + // Arrange + var scope = new TestViewBufferScope(4); + + var original = new ViewBuffer(scope, "original"); + var other = new ViewBuffer(scope, "other"); + + other.Append("Hi"); + + var page = other.Pages[0]; + + // Act + original.AppendHtml(other); + + // Assert + Assert.Empty(other.Pages); // Page was taken + Assert.Same(page, Assert.Single(original.Pages)); + } + + [Fact] + public void AppendHtml_ViewBuffer_TakesPage_IfCurrentPageInOriginalIsFull() + { + // Arrange + var scope = new TestViewBufferScope(4); + + var original = new ViewBuffer(scope, "original"); + var other = new ViewBuffer(scope, "other"); + + for (var i = 0; i < 4; i++) + { + original.Append($"original-{i}"); + } + + other.Append("Hi"); + + var page = other.Pages[0]; + + // Act + original.AppendHtml(other); + + // Assert + Assert.Empty(other.Pages); // Page was taken + Assert.Equal(2, original.Pages.Count); + Assert.Same(page, original.Pages[1]); + } + + [Fact] + public void AppendHtml_ViewBuffer_TakesPage_IfCurrentPageDoesNotHaveCapacity() + { + // Arrange + var scope = new TestViewBufferScope(4); + + var original = new ViewBuffer(scope, "original"); + var other = new ViewBuffer(scope, "other"); + + for (var i = 0; i < 3; i++) + { + original.Append($"original-{i}"); + } + + // With two items, we'd try to copy the items, but there's no room in the current page. + // So we just take over the page. + for (var i = 0; i < 2; i++) + { + other.Append($"other-{i}"); + } + + var page = other.Pages[0]; + + // Act + original.AppendHtml(other); + + // Assert + Assert.Empty(other.Pages); // Page was taken + Assert.Equal(2, original.Pages.Count); + Assert.Same(page, original.Pages[1]); + } + + [Fact] + public void AppendHtml_ViewBuffer_CopiesItems_IfCurrentPageHasRoom() + { + // Arrange + var scope = new TestViewBufferScope(4); + + var original = new ViewBuffer(scope, "original"); + var other = new ViewBuffer(scope, "other"); + + for (var i = 0; i < 2; i++) + { + original.Append($"original-{i}"); + } + + // With two items, this is half full so we try to copy the items. + for (var i = 0; i < 2; i++) + { + other.Append($"other-{i}"); + } + + var page = other.Pages[0]; + + // Act + original.AppendHtml(other); + + // Assert + Assert.Empty(other.Pages); // Other is cleared + Assert.Contains(page.Buffer, scope.ReturnedBuffers); // Buffer was returned + + Assert.Collection( + Assert.Single(original.Pages).Buffer, + item => Assert.Equal("original-0", item.Value), + item => Assert.Equal("original-1", item.Value), + item => Assert.Equal("other-0", item.Value), + item => Assert.Equal("other-1", item.Value)); + } + + [Fact] + public void AppendHtml_ViewBuffer_CanAddToTakenPage() + { + // Arrange + var scope = new TestViewBufferScope(4); + + var original = new ViewBuffer(scope, "original"); + var other = new ViewBuffer(scope, "other"); + + for (var i = 0; i < 3; i++) + { + original.Append($"original-{i}"); + } + + // More than half full, so we take the page + for (var i = 0; i < 3; i++) + { + other.Append($"other-{i}"); + } + + var page = other.Pages[0]; + original.AppendHtml(other); + + // Act + original.Append("after-merge"); + + // Assert + Assert.Empty(other.Pages); // Other is cleared + + Assert.Equal(2, original.Pages.Count); + Assert.Collection( + original.Pages[0].Buffer, + item => Assert.Equal("original-0", item.Value), + item => Assert.Equal("original-1", item.Value), + item => Assert.Equal("original-2", item.Value), + item => Assert.Null(item.Value)); + Assert.Collection( + original.Pages[1].Buffer, + item => Assert.Equal("other-0", item.Value), + item => Assert.Equal("other-1", item.Value), + item => Assert.Equal("other-2", item.Value), + item => Assert.Equal("after-merge", item.Value)); + } + + [Fact] + public void AppendHtml_ViewBuffer_MultiplePages() + { + // Arrange + var scope = new TestViewBufferScope(4); + + var original = new ViewBuffer(scope, "original"); + var other = new ViewBuffer(scope, "other"); + + for (var i = 0; i < 2; i++) + { + original.Append($"original-{i}"); + } + + for (var i = 0; i < 9; i++) + { + other.Append($"other-{i}"); + } + + var pages = new List(other.Pages); + + // Act + original.AppendHtml(other); + + // Assert + Assert.Empty(other.Pages); // Other is cleared + + Assert.Equal(4, original.Pages.Count); + Assert.Collection( + original.Pages[0].Buffer, + item => Assert.Equal("original-0", item.Value), + item => Assert.Equal("original-1", item.Value), + item => Assert.Null(item.Value), + item => Assert.Null(item.Value)); + Assert.Collection( + original.Pages[1].Buffer, + item => Assert.Equal("other-0", item.Value), + item => Assert.Equal("other-1", item.Value), + item => Assert.Equal("other-2", item.Value), + item => Assert.Equal("other-3", item.Value)); + Assert.Collection( + original.Pages[2].Buffer, + item => Assert.Equal("other-4", item.Value), + item => Assert.Equal("other-5", item.Value), + item => Assert.Equal("other-6", item.Value), + item => Assert.Equal("other-7", item.Value)); + Assert.Collection( + original.Pages[3].Buffer, + item => Assert.Equal("other-8", item.Value), + item => Assert.Null(item.Value), + item => Assert.Null(item.Value), + item => Assert.Null(item.Value)); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewBufferTextWriterTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewBufferTextWriterTest.cs new file mode 100644 index 0000000000..0ff27fc1c1 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewBufferTextWriterTest.cs @@ -0,0 +1,176 @@ +// 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.Buffers; +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal +{ + public class ViewBufferTextWriterTest + { + private static readonly char[] Content; + + static ViewBufferTextWriterTest() + { + Content = new char[4 * ViewBufferTextWriter.PageSize]; + for (var i = 0; i < Content.Length; i++) + { + Content[i] = (char)((i % 26) + 'A'); + } + } + + [Fact] + public async Task Write_Char() + { + // Arrange + var pool = new TestArrayPool(); + var inner = new StringWriter(); + + var writer = new ViewBufferTextWriter(pool, inner); + + // Act + for (var i = 0; i < Content.Length; i++) + { + writer.Write(Content[i]); + } + + await writer.FlushAsync(); + + // Assert + Assert.Equal(Content, inner.ToString().ToCharArray()); + } + + [Fact] + public async Task Write_CharArray() + { + // Arrange + var pool = new TestArrayPool(); + var inner = new StringWriter(); + + var writer = new ViewBufferTextWriter(pool, inner); + + // These numbers chosen to hit boundary conditions in buffer lengths + Assert.Equal(4096, Content.Length); // Update these numbers if this changes. + var chunkSizes = new int[] { 3, 1021, 1023, 1023, 1, 1, 1024 }; + + // Act + var offset = 0; + foreach (var chunkSize in chunkSizes) + { + var chunk = new char[chunkSize]; + for (var j = 0; j < chunkSize; j++) + { + chunk[j] = Content[offset + j]; + } + + writer.Write(chunk); + offset += chunkSize; + } + + await writer.FlushAsync(); + + // Assert + var array = inner.ToString().ToCharArray(); + for (var i = 0; i < Content.Length; i++) + { + Assert.Equal(Content[i], array[i]); + } + + Assert.Equal(Content, inner.ToString().ToCharArray()); + } + + [Fact] + public async Task Write_CharArray_Bounded() + { + // Arrange + var pool = new TestArrayPool(); + var inner = new StringWriter(); + + var writer = new ViewBufferTextWriter(pool, inner); + + // These numbers chosen to hit boundary conditions in buffer lengths + Assert.Equal(4096, Content.Length); // Update these numbers if this changes. + var chunkSizes = new int[] { 3, 1021, 1023, 1023, 1, 1, 1024 }; + + // Act + var offset = 0; + foreach (var chunkSize in chunkSizes) + { + writer.Write(Content, offset, chunkSize); + offset += chunkSize; + } + + await writer.FlushAsync(); + + // Assert + Assert.Equal(Content, inner.ToString().ToCharArray()); + } + + [Fact] + public async Task Write_String() + { + // Arrange + var pool = new TestArrayPool(); + var inner = new StringWriter(); + + var writer = new ViewBufferTextWriter(pool, inner); + + // These numbers chosen to hit boundary conditions in buffer lengths + Assert.Equal(4096, Content.Length); // Update these numbers if this changes. + var chunkSizes = new int[] { 3, 1021, 1023, 1023, 1, 1, 1024 }; + + // Act + var offset = 0; + foreach (var chunkSize in chunkSizes) + { + var chunk = new string(Content, offset, chunkSize); + writer.Write(chunk); + offset += chunkSize; + } + + await writer.FlushAsync(); + + // Assert + Assert.Equal(Content, inner.ToString().ToCharArray()); + } + + [Fact] + public async Task FlushAsync_ReturnsPages() + { + // Arrange + var pool = new TestArrayPool(); + var inner = new StringWriter(); + + var writer = new ViewBufferTextWriter(pool, inner); + + for (var i = 0; i < Content.Length; i++) + { + writer.Write(Content[i]); + } + + // Act + await writer.FlushAsync(); + + // Assert + Assert.Equal(3, pool.Returned.Count); + } + + private class TestArrayPool: ArrayPool + { + public IList Returned { get; } = new List(); + + public override char[] Rent(int minimumLength) + { + return new char[minimumLength]; + } + + public override void Return(char[] buffer, bool clearArray = false) + { + Returned.Add(buffer); + } + } + } +}