Less allocations in ViewBuffer when there is only one ViewBufferPage

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewBuffer.cs

@@ -25,6 +25,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
         private readonly IViewBufferScope _bufferScope;
         private readonly string _name;
         private readonly int _pageSize;
+        private ViewBufferPage _currentPage;         // Limits allocation if the ViewBuffer has only one page (frequent case).
+        private List<ViewBufferPage> _multiplePages; // Allocated only if necessary
 
         /// <summary>
         /// Initializes a new instance of <see cref="ViewBuffer"/>.
@@ -47,14 +49,45 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             _bufferScope = bufferScope;
             _name = name;
             _pageSize = pageSize;
-
-            Pages = new List<ViewBufferPage>();
         }
 
         /// <summary>
-        /// Gets the backing buffer.
+        /// Get the <see cref="ViewBufferPage"/> count.
         /// </summary>
-        public IList<ViewBufferPage> Pages { get; }
+        public int Count
+        {
+            get
+            {
+                if (_multiplePages != null)
+                {
+                    return _multiplePages.Count;
+                }
+                if (_currentPage != null)
+                {
+                    return 1;
+                }
+                return 0;
+            }
+        }
+
+        /// <summary>
+        /// Gets a <see cref="ViewBufferPage"/>.
+        /// </summary>
+        public ViewBufferPage this[int index]
+        {
+            get
+            {
+                if (_multiplePages != null)
+                {
+                    return _multiplePages[index];
+                }
+                if (index == 0 && _currentPage != null)
+                {
+                    return _currentPage;
+                }
+                throw new IndexOutOfRangeException();
+            }
+        }
 
         /// <inheritdoc />
         public IHtmlContentBuilder Append(string unencoded)
@@ -102,33 +135,34 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
 
         private ViewBufferPage GetCurrentPage()
         {
-            ViewBufferPage page;
+            if (_currentPage == null || _currentPage.IsFull)
-            if (Pages.Count == 0)
             {
-                page = new ViewBufferPage(_bufferScope.GetPage(_pageSize));
+                AddPage(new ViewBufferPage(_bufferScope.GetPage(_pageSize)));
-                Pages.Add(page);
             }
-            else
+            return _currentPage;
+        }
+
+        private void AddPage(ViewBufferPage page)
+        {
+            if (_multiplePages != null)
             {
-                page = Pages[Pages.Count - 1];
+                _multiplePages.Add(page);
-                if (page.IsFull)
+            }
-                {
+            else if (_currentPage != null)
-                    page = new ViewBufferPage(_bufferScope.GetPage(_pageSize));
+            {
-                    Pages.Add(page);
+                _multiplePages = new List<ViewBufferPage>(2);
-                }
+                _multiplePages.Add(_currentPage);
+                _multiplePages.Add(page);
             }
 
-            return page;
+            _currentPage = page;
         }
 
         /// <inheritdoc />
         public IHtmlContentBuilder Clear()
         {
-            if (Pages != null)
+            _multiplePages = null;
-            {
+            _currentPage = null;
-                Pages.Clear();
-            }
-
             return this;
         }
 
@@ -145,14 +179,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 throw new ArgumentNullException(nameof(encoder));
             }
 
-            if (Pages == null)
+            for (var i = 0; i < Count; i++)
             {
-                return;
+                var page = this[i];
-            }
-
-            for (var i = 0; i < Pages.Count; i++)
-            {
-                var page = Pages[i];
                 for (var j = 0; j < page.Count; j++)
                 {
                     var value = page.Buffer[j];
@@ -192,14 +221,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 throw new ArgumentNullException(nameof(encoder));
             }
 
-            if (Pages == null)
+            for (var i = 0; i < Count; i++)
             {
-                return;
+                var page = this[i];
-            }
-
-            for (var i = 0; i < Pages.Count; i++)
-            {
-                var page = Pages[i];
                 for (var j = 0; j < page.Count; j++)
                 {
                     var value = page.Buffer[j];
@@ -238,14 +262,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 throw new ArgumentNullException(nameof(destination));
             }
 
-            if (Pages == null)
+            for (var i = 0; i < Count; i++)
             {
-                return;
+                var page = this[i];
-            }
-
-            for (var i = 0; i < Pages.Count; i++)
-            {
-                var page = Pages[i];
                 for (var j = 0; j < page.Count; j++)
                 {
                     var value = page.Buffer[j];
@@ -275,11 +294,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 throw new ArgumentNullException(nameof(destination));
             }
 
-            if (Pages == null)
-            {
-                return;
-            }
-
             // Perf: We have an efficient implementation when the destination is another view buffer,
             // we can just insert our pages as-is.
             var other = destination as ViewBuffer;
@@ -289,9 +303,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 return;
             }
 
-            for (var i = 0; i < Pages.Count; i++)
+            for (var i = 0; i < Count; i++)
             {
-                var page = Pages[i];
+                var page = this[i];
                 for (var j = 0; j < page.Count; j++)
                 {
                     var value = page.Buffer[j];
@@ -313,23 +327,23 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 }
             }
 
-            for (var i = 0; i < Pages.Count; i++)
+            for (var i = 0; i < Count; i++)
             {
-                var page = Pages[i];
+                var page = this[i];
                 Array.Clear(page.Buffer, 0, page.Count);
                 _bufferScope.ReturnSegment(page.Buffer);
             }
 
-            Pages.Clear();
+            Clear();
         }
 
         private void MoveTo(ViewBuffer destination)
         {
-            for (var i = 0; i < Pages.Count; i++)
+            for (var i = 0; i < Count; i++)
             {
-                var page = Pages[i];
+                var page = this[i];
 
-                var destinationPage = destination.Pages.Count == 0 ? null : destination.Pages[destination.Pages.Count - 1];
+                var destinationPage = destination.Count == 0 ? null : destination[destination.Count - 1];
 
                 // If the source page is less or equal to than half full, let's copy it's content to the destination
                 // page if possible.
@@ -356,12 +370,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 else
                 {
                     // Otherwise, let's just add the source page to the other buffer.
-                    destination.Pages.Add(page);
+                    destination.AddPage(page);
                 }
 
             }
 
-            Pages.Clear();
+            Clear();
         }
 
         private class EncodingWrapper : IHtmlContent

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ViewBufferTest.cs

@@ -25,7 +25,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             buffer.Append("Hello world");
 
             // Assert
-            var page = Assert.Single(buffer.Pages);
+            Assert.Equal(1, buffer.Count);
+            var page = buffer[0];
             Assert.Equal(1, page.Count);
             Assert.IsAssignableFrom<IHtmlContent>(page.Buffer[0].Value);
         }
@@ -41,7 +42,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             buffer.AppendHtml(content);
 
             // Assert
-            var page = Assert.Single(buffer.Pages);
+            Assert.Equal(1, buffer.Count);
+            var page = buffer[0];
             Assert.Equal(1, page.Count);
             Assert.Same(content, page.Buffer[0].Value);
         }
@@ -57,13 +59,32 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             buffer.AppendHtml(value);
 
             // Assert
-            var page = Assert.Single(buffer.Pages);
+            Assert.Equal(1, buffer.Count);
+            var page = buffer[0];
             Assert.Equal(1, page.Count);
             Assert.Equal("Hello world", Assert.IsType<string>(page.Buffer[0].Value));
         }
 
         [Fact]
-        public void Append_CreatesNewPages_WhenCurrentPageIsFull()
+        public void Append_CreatesOnePage()
+        {
+            // Arrange
+            var buffer = new ViewBuffer(new TestViewBufferScope(), "some-name", pageSize: 32);
+            var expected = Enumerable.Range(0, 32).Select(i => i.ToString());
+
+            // Act
+            foreach (var item in expected)
+            {
+                buffer.AppendHtml(item);
+            }
+
+            // Assert
+            Assert.Equal(1, buffer.Count);
+            Assert.Equal(expected, buffer[0].Buffer.Select(v => v.Value));
+        }
+
+        [Fact]
+        public void Append_CreatesTwoPages()
         {
             // Arrange
             var buffer = new ViewBuffer(new TestViewBufferScope(), "some-name", pageSize: 32);
@@ -78,8 +99,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             buffer.AppendHtml("world");
 
             // Assert
-            Assert.Equal(2, buffer.Pages.Count);
+            Assert.Equal(2, buffer.Count);
-            Assert.Collection(buffer.Pages,
+            Assert.Collection(new[] { buffer[0], buffer[1] },
                 page => Assert.Equal(expected, page.Buffer.Select(v => v.Value)),
                 page =>
                 {
@@ -89,9 +110,43 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 });
         }
 
+        [Fact]
+        public void Append_CreatesManyPages()
+        {
+            // Arrange
+            var buffer = new ViewBuffer(new TestViewBufferScope(), "some-name", pageSize: 32);
+            var expected0 = Enumerable.Range(0, 32).Select(i => i.ToString());
+            var expected1 = Enumerable.Range(32, 32).Select(i => i.ToString());
+
+            // Act
+            foreach (var item in expected0)
+            {
+                buffer.AppendHtml(item);
+            }
+            foreach (var item in expected1)
+            {
+                buffer.AppendHtml(item);
+            }
+            buffer.AppendHtml("Hello");
+            buffer.AppendHtml("world");
+
+            // Assert
+            Assert.Equal(3, buffer.Count);
+            Assert.Collection(new[] { buffer[0], buffer[1], buffer[2] },
+                page => Assert.Equal(expected0, page.Buffer.Select(v => v.Value)),
+                page => Assert.Equal(expected1, page.Buffer.Select(v => v.Value)),
+                page =>
+                {
+                    var array = page.Buffer;
+                    Assert.Equal("Hello", array[0].Value);
+                    Assert.Equal("world", array[1].Value);
+                });
+        }
+
         [Theory]
-        [InlineData(1)]
+        [InlineData(1)]             // Create one page before clear
-        [InlineData(35)]
+        [InlineData(35)]            // Create two pages before clear
+        [InlineData(65)]            // Create many pages before clear
         public void Clear_ResetsBackingBufferAndIndex(int valuesToWrite)
         {
             // Arrange
@@ -106,7 +161,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             buffer.AppendHtml("world");
 
             // Assert
-            var page = Assert.Single(buffer.Pages);
+            Assert.Equal(1, buffer.Count);
+            var page = buffer[0];
             Assert.Equal(1, page.Count);
             Assert.Equal("world", page.Buffer[0].Value);
         }
@@ -211,7 +267,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             buffer.CopyTo(destination);
 
             // Assert
-            Assert.Same(nested, buffer.Pages[0].Buffer[0].Value);
+            Assert.Same(nested, buffer[0].Buffer[0].Value);
             Assert.Equal("Hello", Assert.IsType<HtmlString>(nestedItems[0]).Value);
             Assert.Equal("Hello", Assert.IsType<HtmlString>(destinationItems[0]).Value);
         }
@@ -235,7 +291,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
 
             // Assert
             Assert.Empty(nestedItems);
-            Assert.Empty(buffer.Pages);
+            Assert.Equal(0, buffer.Count);
             Assert.Equal("Hello", Assert.IsType<HtmlString>(destinationItems[0]).Value);
         }
 
@@ -250,14 +306,15 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
 
             other.AppendHtml("Hi");
 
-            var page = other.Pages[0];
+            var page = other[0];
 
             // Act
             other.MoveTo(original);
 
             // Assert
-            Assert.Empty(other.Pages); // Page was taken
+            Assert.Equal(0, other.Count); // Page was taken
-            Assert.Same(page, Assert.Single(original.Pages));
+            Assert.Equal(1, original.Count);
+            Assert.Same(page, original[0]);
         }
 
         [Fact]
@@ -276,15 +333,15 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
 
             other.AppendHtml("Hi");
 
-            var page = other.Pages[0];
+            var page = other[0];
 
             // Act
             other.MoveTo(original);
 
             // Assert
-            Assert.Empty(other.Pages); // Page was taken
+            Assert.Equal(0, other.Count); // Page was taken
-            Assert.Equal(2, original.Pages.Count);
+            Assert.Equal(2, original.Count);
-            Assert.Same(page, original.Pages[1]);
+            Assert.Same(page, original[1]);
         }
 
         [Fact]
@@ -308,15 +365,15 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 other.AppendHtml($"other-{i}");
             }
 
-            var page = other.Pages[0];
+            var page = other[0];
 
             // Act
             other.MoveTo(original);
 
             // Assert
-            Assert.Empty(other.Pages); // Page was taken
+            Assert.Equal(0, other.Count); // Page was taken
-            Assert.Equal(2, original.Pages.Count);
+            Assert.Equal(2, original.Count);
-            Assert.Same(page, original.Pages[1]);
+            Assert.Same(page, original[1]);
         }
 
         [Fact]
@@ -339,17 +396,18 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 other.AppendHtml($"other-{i}");
             }
 
-            var page = other.Pages[0];
+            var page = other[0];
 
             // Act
             other.MoveTo(original);
 
             // Assert
-            Assert.Empty(other.Pages); // Other is cleared
+            Assert.Equal(0, other.Count); // Other is cleared
             Assert.Contains(page.Buffer, scope.ReturnedBuffers); // Buffer was returned
 
+            Assert.Equal(1, original.Count);
             Assert.Collection(
-                Assert.Single(original.Pages).Buffer,
+                original[0].Buffer,
                 item => Assert.Equal("original-0", item.Value),
                 item => Assert.Equal("original-1", item.Value),
                 item => Assert.Equal("other-0", item.Value),
@@ -376,24 +434,24 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 other.AppendHtml($"other-{i}");
             }
 
-            var page = other.Pages[0];
+            var page = other[0];
             other.MoveTo(original);
 
             // Act
             original.AppendHtml("after-merge");
 
             // Assert
-            Assert.Empty(other.Pages); // Other is cleared
+            Assert.Equal(0, other.Count); // Other is cleared
 
-            Assert.Equal(2, original.Pages.Count);
+            Assert.Equal(2, original.Count);
             Assert.Collection(
-                original.Pages[0].Buffer,
+                original[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,
+                original[1].Buffer,
                 item => Assert.Equal("other-0", item.Value),
                 item => Assert.Equal("other-1", item.Value),
                 item => Assert.Equal("other-2", item.Value),
@@ -419,35 +477,39 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
                 other.AppendHtml($"other-{i}");
             }
 
-            var pages = new List<ViewBufferPage>(other.Pages);
+            var pages = new List<ViewBufferPage>();
+            for (var i = 0; i < other.Count; i++)
+            {
+                pages.Add(other[i]);
+            }
 
             // Act
             other.MoveTo(original);
 
             // Assert
-            Assert.Empty(other.Pages); // Other is cleared
+            Assert.Equal(0, other.Count); // Other is cleared
 
-            Assert.Equal(4, original.Pages.Count);
+            Assert.Equal(4, original.Count);
             Assert.Collection(
-                original.Pages[0].Buffer,
+                original[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,
+                original[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,
+                original[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,
+                original[3].Buffer,
                 item => Assert.Equal("other-8", item.Value),
                 item => Assert.Null(item.Value),
                 item => Assert.Null(item.Value),

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/ViewBufferTextWriterTest.cs

@@ -60,7 +60,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             writer.Write(2.718m);
 
             // Assert
-            Assert.Empty(buffer.Pages);
+            Assert.Equal(0, buffer.Count);
             foreach (var item in expected)
             {
                 inner.Verify(v => v.Write(item), Times.Once());
@@ -220,7 +220,13 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
 
         private static object[] GetValues(ViewBuffer buffer)
         {
-            return buffer.Pages
+            var pages = new List<ViewBufferPage>();
+            for (var i = 0; i < buffer.Count; i++)
+            {
+                pages.Add(buffer[i]);
+            }
+
+            return pages
                 .SelectMany(c => c.Buffer)
                 .Select(d => d.Value)
                 .TakeWhile(d => d != null)