From 22a959a503bd5e0a4506d2683449943761aa8781 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 18 Jul 2019 17:25:27 -0700 Subject: [PATCH] Pool byte[] in RemoteRenderer (#11976) * Pool byte[] in RemoteRenderer --- .../Microsoft.AspNetCore.Components.csproj | 1 + .../src/RenderTree/ArrayBuilderExtensions.cs | 27 ++++ .../src/BlazorPack/BlazorPackHubProtocol.cs | 4 +- .../src/Circuits/ArrayBuilderMemoryStream.cs | 125 ++++++++++++++++++ .../Server/src/Circuits/RemoteRenderer.cs | 72 ++++++---- .../Server/src/Circuits/RenderBatchWriter.cs | 15 ++- ...rosoft.AspNetCore.Components.Server.csproj | 3 +- .../test/Circuits/RenderBatchWriterTest.cs | 3 +- .../RenderTree => Shared/src}/ArrayBuilder.cs | 20 +-- .../Ignitor.Test/RenderBatchReaderTest.cs | 5 +- 10 files changed, 217 insertions(+), 58 deletions(-) create mode 100644 src/Components/Components/src/RenderTree/ArrayBuilderExtensions.cs create mode 100644 src/Components/Server/src/Circuits/ArrayBuilderMemoryStream.cs rename src/Components/{Components/src/RenderTree => Shared/src}/ArrayBuilder.cs (87%) diff --git a/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj b/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj index 6c773e9d42..dffcf43a97 100644 --- a/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj +++ b/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Components/Components/src/RenderTree/ArrayBuilderExtensions.cs b/src/Components/Components/src/RenderTree/ArrayBuilderExtensions.cs new file mode 100644 index 0000000000..3fb698d3ee --- /dev/null +++ b/src/Components/Components/src/RenderTree/ArrayBuilderExtensions.cs @@ -0,0 +1,27 @@ +// 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; + +namespace Microsoft.AspNetCore.Components.RenderTree +{ + internal static class ArrayBuilderExtensions + { + /// + /// Produces an structure describing the current contents. + /// + /// The . + public static ArrayRange ToRange(this ArrayBuilder builder) + => new ArrayRange(builder.Buffer, builder.Count); + + /// + /// Produces an structure describing the selected contents. + /// + /// The + /// The index of the first item in the segment. + /// One plus the index of the last item in the segment. + /// The . + public static ArrayBuilderSegment ToSegment(this ArrayBuilder builder, int fromIndexInclusive, int toIndexExclusive) + => new ArrayBuilderSegment(builder, fromIndexInclusive, toIndexExclusive - fromIndexInclusive); + } +} diff --git a/src/Components/Server/src/BlazorPack/BlazorPackHubProtocol.cs b/src/Components/Server/src/BlazorPack/BlazorPackHubProtocol.cs index ea97e65cdc..efebec9920 100644 --- a/src/Components/Server/src/BlazorPack/BlazorPackHubProtocol.cs +++ b/src/Components/Server/src/BlazorPack/BlazorPackHubProtocol.cs @@ -424,8 +424,8 @@ namespace Microsoft.AspNetCore.Components.Server.BlazorPack writer.Write(floatValue); break; - case byte[] byteArray: - writer.Write(byteArray); + case ArraySegment bytes: + writer.Write(bytes); break; case Exception exception: diff --git a/src/Components/Server/src/Circuits/ArrayBuilderMemoryStream.cs b/src/Components/Server/src/Circuits/ArrayBuilderMemoryStream.cs new file mode 100644 index 0000000000..ff1ff63114 --- /dev/null +++ b/src/Components/Server/src/Circuits/ArrayBuilderMemoryStream.cs @@ -0,0 +1,125 @@ +// 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.IO; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Components.Web.Rendering +{ + /// + /// Writeable memory stream backed by a an . + /// + internal sealed class ArrayBuilderMemoryStream : Stream + { + public ArrayBuilderMemoryStream(ArrayBuilder arrayBuilder) + { + ArrayBuilder = arrayBuilder; + } + + /// + public override bool CanRead => false; + + /// + public override bool CanSeek => false; + + /// + public override bool CanWrite => true; + + /// + public override long Length => ArrayBuilder.Count; + + /// + public override long Position + { + get => ArrayBuilder.Count; + set => throw new NotSupportedException(); + } + + public ArrayBuilder ArrayBuilder { get; } + + /// + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + /// + public override int Read(byte[] buffer, int offset, int count) + => throw new NotSupportedException(); + + /// + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + => throw new NotSupportedException(); + + /// + public override void Write(byte[] buffer, int offset, int count) + { + ValidateArguments(buffer, offset, count); + + ArrayBuilder.Append(buffer, offset, count); + } + + /// + public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + ValidateArguments(buffer, offset, count); + + ArrayBuilder.Append(buffer, offset, count); + return Task.CompletedTask; + } + + /// + public override void Flush() + { + // Do nothing. + } + + /// + public override Task FlushAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + /// + public override void SetLength(long value) => throw new NotSupportedException(); + + /// + protected override void Dispose(bool disposing) + { + // Do nothing. + } + + /// + public override ValueTask DisposeAsync() => default; + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void ValidateArguments(byte[] buffer, int offset, int count) + { + if (buffer == null) + { + ThrowHelper.ThrowArgumentNullException(nameof(buffer)); + } + + if (offset < 0) + { + ThrowHelper.ThrowArgumentOutOfRangeException(nameof(offset)); + } + + if (count < 0) + { + ThrowHelper.ThrowArgumentOutOfRangeException(nameof(count)); + } + + if (buffer.Length - offset < count) + { + ThrowHelper.ThrowArgumentOutOfRangeException(nameof(offset)); + } + } + + private static class ThrowHelper + { + public static void ThrowArgumentNullException(string name) + => throw new ArgumentNullException(name); + + public static void ThrowArgumentOutOfRangeException(string name) + => throw new ArgumentOutOfRangeException(name); + } + } +} diff --git a/src/Components/Server/src/Circuits/RemoteRenderer.cs b/src/Components/Server/src/Circuits/RemoteRenderer.cs index 8065ca4afd..dd12dce2aa 100644 --- a/src/Components/Server/src/Circuits/RemoteRenderer.cs +++ b/src/Components/Server/src/Circuits/RemoteRenderer.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Concurrent; -using System.IO; using System.Linq; using System.Text.Encodings.Web; using System.Threading; @@ -102,12 +101,13 @@ namespace Microsoft.AspNetCore.Components.Web.Rendering protected override void Dispose(bool disposing) { _disposing = true; - base.Dispose(true); + _rendererRegistry.TryRemove(Id); while (PendingRenderBatches.TryDequeue(out var entry)) { entry.CompletionSource.TrySetCanceled(); + entry.Data.Dispose(); } - _rendererRegistry.TryRemove(Id); + base.Dispose(true); } /// @@ -123,30 +123,34 @@ namespace Microsoft.AspNetCore.Components.Web.Rendering // SignalR's SendAsync can wait an arbitrary duration before serializing the params. // The RenderBatch buffer will get reused by subsequent renders, so we need to // snapshot its contents now. - // TODO: Consider using some kind of array pool instead of allocating a new - // buffer on every render. - byte[] batchBytes; - using (var memoryStream = new MemoryStream()) + var arrayBuilder = new ArrayBuilder(2048); + using var memoryStream = new ArrayBuilderMemoryStream(arrayBuilder); + PendingRender pendingRender; + try { using (var renderBatchWriter = new RenderBatchWriter(memoryStream, false)) { renderBatchWriter.Write(in batch); } - batchBytes = memoryStream.ToArray(); + var renderId = Interlocked.Increment(ref _nextRenderId); + + pendingRender = new PendingRender( + renderId, + arrayBuilder, + new TaskCompletionSource()); + + // Buffer the rendered batches no matter what. We'll send it down immediately when the client + // is connected or right after the client reconnects. + + PendingRenderBatches.Enqueue(pendingRender); + } + catch + { + // if we throw prior to queueing the write, dispose the builder. + arrayBuilder.Dispose(); + throw; } - - var renderId = Interlocked.Increment(ref _nextRenderId); - - var pendingRender = new PendingRender( - renderId, - batchBytes, - new TaskCompletionSource()); - - // Buffer the rendered batches no matter what. We'll send it down immediately when the client - // is connected or right after the client reconnects. - - PendingRenderBatches.Enqueue(pendingRender); // Fire and forget the initial send for this batch (if connected). Otherwise it will be sent // as soon as the client reconnects. @@ -181,8 +185,9 @@ namespace Microsoft.AspNetCore.Components.Web.Rendering return; } - Log.BeginUpdateDisplayAsync(_logger, _client.ConnectionId, pending.BatchId, pending.Data.Length); - await _client.SendAsync("JS.RenderBatch", Id, pending.BatchId, pending.Data); + Log.BeginUpdateDisplayAsync(_logger, _client.ConnectionId, pending.BatchId, pending.Data.Count); + var segment = new ArraySegment(pending.Data.Buffer, 0, pending.Data.Count); + await _client.SendAsync("JS.RenderBatch", Id, pending.BatchId, segment); } catch (Exception e) { @@ -207,22 +212,33 @@ namespace Microsoft.AspNetCore.Components.Web.Rendering // line (i.e., matching the order in which we received batch completion messages) based on the fact that SignalR // synchronizes calls to hub methods. That is, it won't issue more than one call to this method from the same hub // at the same time on different threads. - if (!PendingRenderBatches.TryDequeue(out var entry) || entry.BatchId != incomingBatchId) + if (!PendingRenderBatches.TryDequeue(out var entry)) { HandleException( new InvalidOperationException($"Received a notification for a rendered batch when not expecting it. Batch id '{incomingBatchId}'.")); } else { - if (errorMessageOrNull == null) + entry.Data.Dispose(); + + if (entry.BatchId != incomingBatchId) { - Log.CompletingBatchWithoutError(_logger, entry.BatchId); + HandleException( + new InvalidOperationException($"Received a notification for a rendered batch when not expecting it. Batch id '{incomingBatchId}'.")); } else { - Log.CompletingBatchWithError(_logger, entry.BatchId, errorMessageOrNull); + if (errorMessageOrNull == null) + { + Log.CompletingBatchWithoutError(_logger, entry.BatchId); + } + else + { + Log.CompletingBatchWithError(_logger, entry.BatchId, errorMessageOrNull); + } } + CompleteRender(entry.CompletionSource, errorMessageOrNull); } } @@ -241,7 +257,7 @@ namespace Microsoft.AspNetCore.Components.Web.Rendering internal readonly struct PendingRender { - public PendingRender(long batchId, byte[] data, TaskCompletionSource completionSource) + public PendingRender(long batchId, ArrayBuilder data, TaskCompletionSource completionSource) { BatchId = batchId; Data = data; @@ -249,7 +265,7 @@ namespace Microsoft.AspNetCore.Components.Web.Rendering } public long BatchId { get; } - public byte[] Data { get; } + public ArrayBuilder Data { get; } public TaskCompletionSource CompletionSource { get; } } diff --git a/src/Components/Server/src/Circuits/RenderBatchWriter.cs b/src/Components/Server/src/Circuits/RenderBatchWriter.cs index ef035de4ba..f71da86e59 100644 --- a/src/Components/Server/src/Circuits/RenderBatchWriter.cs +++ b/src/Components/Server/src/Circuits/RenderBatchWriter.cs @@ -1,14 +1,14 @@ // 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 Microsoft.AspNetCore.Components.Rendering; -using Microsoft.AspNetCore.Components.RenderTree; using System; using System.Collections.Generic; using System.IO; using System.Text; +using Microsoft.AspNetCore.Components.Rendering; +using Microsoft.AspNetCore.Components.RenderTree; -namespace Microsoft.AspNetCore.Components.Server.Circuits +namespace Microsoft.AspNetCore.Components.Web.Rendering { // TODO: We should consider *not* having this type of infrastructure in the .Server // project, but instead in some new project called .Remote or similar, since it @@ -33,13 +33,13 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits /// internal class RenderBatchWriter : IDisposable { - private readonly List _strings; + private readonly ArrayBuilder _strings; private readonly Dictionary _deduplicatedStringIndices; private readonly BinaryWriter _binaryWriter; public RenderBatchWriter(Stream output, bool leaveOpen) { - _strings = new List(); + _strings = new ArrayBuilder(); _deduplicatedStringIndices = new Dictionary(); _binaryWriter = new BinaryWriter(output, Encoding.UTF8, leaveOpen); } @@ -243,7 +243,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits if (!allowDeduplication || !_deduplicatedStringIndices.TryGetValue(value, out stringIndex)) { stringIndex = _strings.Count; - _strings.Add(value); + _strings.Append(value); if (allowDeduplication) { @@ -263,7 +263,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits for (var i = 0; i < stringsCount; i++) { - var stringValue = _strings[i]; + var stringValue = _strings.Buffer[i]; locations[i] = (int)_binaryWriter.BaseStream.Position; _binaryWriter.Write(stringValue); } @@ -295,6 +295,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits public void Dispose() { + _strings.Dispose(); _binaryWriter.Dispose(); } } diff --git a/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj b/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj index 734023b0f5..ffa9a5ffb4 100644 --- a/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj +++ b/src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj @@ -8,7 +8,7 @@ true true CS0436;$(NoWarn) - $(DefineConstants);MESSAGEPACK_INTERNAL + $(DefineConstants);MESSAGEPACK_INTERNAL;COMPONENTS_SERVER @@ -28,6 +28,7 @@ + diff --git a/src/Components/Server/test/Circuits/RenderBatchWriterTest.cs b/src/Components/Server/test/Circuits/RenderBatchWriterTest.cs index 3ccba9ff5a..febb5e381d 100644 --- a/src/Components/Server/test/Circuits/RenderBatchWriterTest.cs +++ b/src/Components/Server/test/Circuits/RenderBatchWriterTest.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components.Rendering; using Microsoft.AspNetCore.Components.RenderTree; using Microsoft.AspNetCore.Components.Server.Circuits; +using Microsoft.AspNetCore.Components.Web.Rendering; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using System; @@ -153,7 +154,7 @@ namespace Microsoft.AspNetCore.Components.Server RenderTreeEdit.UpdateMarkup(108, 109), RenderTreeEdit.RemoveAttribute(110, "Some removed attribute"), // To test deduplication }; - var editsBuilder = new ArrayBuilder(); + var editsBuilder = new RenderTree.ArrayBuilder(); editsBuilder.Append(edits, 0, edits.Length); var editsSegment = editsBuilder.ToSegment(1, edits.Length); // Skip first to show offset is respected var bytes = Serialize(new RenderBatch( diff --git a/src/Components/Components/src/RenderTree/ArrayBuilder.cs b/src/Components/Shared/src/ArrayBuilder.cs similarity index 87% rename from src/Components/Components/src/RenderTree/ArrayBuilder.cs rename to src/Components/Shared/src/ArrayBuilder.cs index 9d3e71993a..b0f203fdcc 100644 --- a/src/Components/Components/src/RenderTree/ArrayBuilder.cs +++ b/src/Components/Shared/src/ArrayBuilder.cs @@ -6,7 +6,11 @@ using System.Buffers; using System.Diagnostics; using System.Runtime.CompilerServices; +#if COMPONENTS_SERVER +namespace Microsoft.AspNetCore.Components.Web.Rendering +#else namespace Microsoft.AspNetCore.Components.RenderTree +#endif { /// /// Implements a list that uses an array of objects to store the elements. @@ -154,22 +158,6 @@ namespace Microsoft.AspNetCore.Components.RenderTree _itemsInUse = 0; } - /// - /// Produces an structure describing the current contents. - /// - /// The . - public ArrayRange ToRange() - => new ArrayRange(_items, _itemsInUse); - - /// - /// Produces an structure describing the selected contents. - /// - /// The index of the first item in the segment. - /// One plus the index of the last item in the segment. - /// The . - public ArrayBuilderSegment ToSegment(int fromIndexInclusive, int toIndexExclusive) - => new ArrayBuilderSegment(this, fromIndexInclusive, toIndexExclusive - fromIndexInclusive); - private void GrowBuffer(int desiredCapacity) { var newCapacity = Math.Max(desiredCapacity, _minCapacity); diff --git a/src/Components/test/Ignitor.Test/RenderBatchReaderTest.cs b/src/Components/test/Ignitor.Test/RenderBatchReaderTest.cs index b5e8c35ba4..6d3f996367 100644 --- a/src/Components/test/Ignitor.Test/RenderBatchReaderTest.cs +++ b/src/Components/test/Ignitor.Test/RenderBatchReaderTest.cs @@ -6,11 +6,10 @@ using System.Collections.Generic; using System.IO; using System.Text; using System.Threading.Tasks; -using Castle.Core.Logging; using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components.Rendering; using Microsoft.AspNetCore.Components.RenderTree; -using Microsoft.AspNetCore.Components.Server.Circuits; +using Microsoft.AspNetCore.Components.Web.Rendering; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Xunit; @@ -103,7 +102,7 @@ namespace Ignitor RenderTreeEdit.UpdateMarkup(108, 109), RenderTreeEdit.RemoveAttribute(110, "Some removed attribute"), // To test deduplication }; - var editsBuilder = new ArrayBuilder(); + var editsBuilder = new Microsoft.AspNetCore.Components.RenderTree.ArrayBuilder(); editsBuilder.Append(edits, 0, edits.Length); var editsSegment = editsBuilder.ToSegment(1, edits.Length); // Skip first to show offset is respected var bytes = RoundTripSerialize(new RenderBatch(