Skip to content

[Blazor][Fixes #11964] Limit the amount of pending renders #12763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Aug 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -535,13 +535,13 @@ public abstract partial class Renderer : System.IDisposable
public Renderer(System.IServiceProvider serviceProvider, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { }
public abstract Microsoft.AspNetCore.Components.Dispatcher Dispatcher { get; }
public event System.UnhandledExceptionEventHandler UnhandledSynchronizationException { add { } remove { } }
protected internal virtual void AddToRenderQueue(int componentId, Microsoft.AspNetCore.Components.RenderFragment renderFragment) { }
protected internal int AssignRootComponentId(Microsoft.AspNetCore.Components.IComponent component) { throw null; }
public virtual System.Threading.Tasks.Task DispatchEventAsync(ulong eventHandlerId, Microsoft.AspNetCore.Components.Rendering.EventFieldInfo fieldInfo, System.EventArgs eventArgs) { throw null; }
public void Dispose() { }
protected virtual void Dispose(bool disposing) { }
protected abstract void HandleException(System.Exception exception);
protected Microsoft.AspNetCore.Components.IComponent InstantiateComponent(System.Type componentType) { throw null; }
protected virtual void ProcessPendingRender() { }
protected System.Threading.Tasks.Task RenderRootComponentAsync(int componentId) { throw null; }
[System.Diagnostics.DebuggerStepThroughAttribute]
protected System.Threading.Tasks.Task RenderRootComponentAsync(int componentId, Microsoft.AspNetCore.Components.ParameterView initialParameters) { throw null; }
Expand Down
4 changes: 2 additions & 2 deletions src/Components/Components/src/Rendering/Renderer.Log.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ public static void RenderingComponent(ILogger logger, ComponentState componentSt
}
}

internal static void DisposingComponent(ILogger<Renderer> logger, ComponentState componentState)
public static void DisposingComponent(ILogger<Renderer> logger, ComponentState componentState)
{
if (logger.IsEnabled(LogLevel.Debug)) // This is almost always false, so skip the evaluations
{
_disposingComponent(logger, componentState.ComponentId, componentState.Component.GetType(), null);
}
}

internal static void HandlingEvent(ILogger<Renderer> logger, ulong eventHandlerId, EventArgs eventArgs)
public static void HandlingEvent(ILogger<Renderer> logger, ulong eventHandlerId, EventArgs eventArgs)
{
_handlingEvent(logger, eventHandlerId, eventArgs?.GetType().Name ?? "null", null);
}
Expand Down
21 changes: 18 additions & 3 deletions src/Components/Components/src/Rendering/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo fiel

// Since the task has yielded - process any queued rendering work before we return control
// to the caller.
ProcessRenderQueue();
ProcessPendingRender();
}

// Task completed synchronously or is still running. We already processed all of the rendering
Expand Down Expand Up @@ -334,7 +334,7 @@ internal void AssignEventHandlerId(ref RenderTreeFrame frame)
/// </summary>
/// <param name="componentId">The ID of the component to render.</param>
/// <param name="renderFragment">A <see cref="RenderFragment"/> that will supply the updated UI contents.</param>
protected internal virtual void AddToRenderQueue(int componentId, RenderFragment renderFragment)
internal void AddToRenderQueue(int componentId, RenderFragment renderFragment)
{
EnsureSynchronizationContext();

Expand All @@ -351,7 +351,7 @@ protected internal virtual void AddToRenderQueue(int componentId, RenderFragment

if (!_isBatchInProgress)
{
ProcessRenderQueue();
ProcessPendingRender();
}
}

Expand Down Expand Up @@ -398,13 +398,27 @@ private ComponentState GetOptionalComponentState(int componentId)
? componentState
: null;

/// <summary>
/// Processses pending renders requests from components if there are any.
/// </summary>
protected virtual void ProcessPendingRender()
{
ProcessRenderQueue();
}

private void ProcessRenderQueue()
{
EnsureSynchronizationContext();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can go into ProcessPendingRender, since that's the external entrypoint, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it couldn't because it was on the virtual method, but not that I think again it can, as people is forced to invoke the base to do any meaningful work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change this in a separate PR as I don't want to re-run the build

_isBatchInProgress = true;
var updateDisplayTask = Task.CompletedTask;

try
{
if (_batchBuilder.ComponentRenderQueue.Count == 0)
{
return;
}

// Process render queue until empty
while (_batchBuilder.ComponentRenderQueue.Count > 0)
{
Expand All @@ -423,6 +437,7 @@ private void ProcessRenderQueue()
{
// Ensure we catch errors while running the render functions of the components.
HandleException(e);
return;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a bug. After an exception we should stop everything immediately and not run anything else, like the code after this try..catch..finally block.

}
finally
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public CircuitOptions() { }
public int DisconnectedCircuitMaxRetained { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
public System.TimeSpan DisconnectedCircuitRetentionPeriod { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
public System.TimeSpan JSInteropDefaultCallTimeout { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
public int MaxBufferedUnacknowledgedRenderBatches { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
}
}
namespace Microsoft.AspNetCore.Components.Server.Circuits
Expand Down
12 changes: 12 additions & 0 deletions src/Components/Server/src/CircuitOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,17 @@ public sealed class CircuitOptions
/// Defaults to <c>1 minute</c>.
/// </value>
public TimeSpan JSInteropDefaultCallTimeout { get; set; } = TimeSpan.FromMinutes(1);

/// <summary>
/// Gets or sets the maximum number of render batches that a circuit will buffer until an acknowledgement for the batch is
/// received.
/// </summary>
/// <remarks>
/// When the limit of buffered render batches is reached components will stop rendering and will wait until either the
/// circuit is disconnected and disposed or at least one batch gets acknowledged.
/// </remarks>
/// <value>
/// Defaults to <c>10</c>.</value>
public int MaxBufferedUnacknowledgedRenderBatches { get; set; } = 10;
}
}
2 changes: 1 addition & 1 deletion src/Components/Server/src/Circuits/CircuitHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ await Renderer.Dispatcher.InvokeAsync(() =>
catch (Exception ex)
{
// We don't expect any of this code to actually throw, because DotNetDispatcher.BeginInvoke doesn't throw
// however, we still want this to get logged if we do.
// however, we still want this to get logged if we do.
UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, isTerminating: false));
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/Components/Server/src/Circuits/DefaultCircuitFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@
using System.Linq;
using System.Security.Claims;
using System.Text.Encodings.Web;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Components.Web;
using Microsoft.AspNetCore.Components.Web.Rendering;
using Microsoft.AspNetCore.Components.Routing;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.JSInterop;
using System.Threading.Tasks;

namespace Microsoft.AspNetCore.Components.Server.Circuits
{
Expand All @@ -24,16 +25,19 @@ internal class DefaultCircuitFactory : CircuitFactory
private readonly ILoggerFactory _loggerFactory;
private readonly ILogger _logger;
private readonly CircuitIdFactory _circuitIdFactory;
private readonly CircuitOptions _options;

public DefaultCircuitFactory(
IServiceScopeFactory scopeFactory,
ILoggerFactory loggerFactory,
CircuitIdFactory circuitIdFactory)
CircuitIdFactory circuitIdFactory,
IOptions<CircuitOptions> options)
{
_scopeFactory = scopeFactory ?? throw new ArgumentNullException(nameof(scopeFactory));
_loggerFactory = loggerFactory;
_logger = _loggerFactory.CreateLogger<CircuitFactory>();
_circuitIdFactory = circuitIdFactory ?? throw new ArgumentNullException(nameof(circuitIdFactory));
_options = options.Value;
}

public override CircuitHost CreateCircuitHost(
Expand Down Expand Up @@ -80,6 +84,7 @@ public override CircuitHost CreateCircuitHost(
scope.ServiceProvider,
_loggerFactory,
rendererRegistry,
_options,
jsRuntime,
client,
encoder,
Expand Down
Loading