Skip to content

Commit

Permalink
Various timer cleanup (#3129)
Browse files Browse the repository at this point in the history
This change does 2 things:
- It disables the websocket keep alive since SignalR has its own bidirectional pings. This should remove a significant timer overhead per WebSocket connection that we end up with today. We have a single timer that sends to all connection on an interval.
- Don't pass the CancellationToken to ReadAsync in the handshake since the Pipe implementation holds onto the token for longer than it 
needs to which keeps Timer objects alive (see dotnet/corefx#32806)

I found this when reading the source code and looking at dumps of a couple of SignalR applications.
  • Loading branch information
davidfowl authored Oct 13, 2018
1 parent 5551729 commit defbadb
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace Microsoft.AspNetCore.SignalR
{
public class HubConnectionContext
{
private static readonly Action<object> _cancelReader = state => ((PipeReader)state).CancelPendingRead();
private static readonly WaitCallback _abortedCallback = AbortConnection;

private readonly ConnectionContext _connectionContext;
Expand Down Expand Up @@ -344,7 +345,10 @@ internal async Task<bool> HandshakeAsync(TimeSpan timeout, IReadOnlyList<string>
{
try
{
var input = Input;

using (var cts = new CancellationTokenSource())
using (var registration = cts.Token.Register(_cancelReader, input))
{
if (!Debugger.IsAttached)
{
Expand All @@ -353,7 +357,8 @@ internal async Task<bool> HandshakeAsync(TimeSpan timeout, IReadOnlyList<string>

while (true)
{
var result = await _connectionContext.Transport.Input.ReadAsync(cts.Token);
var result = await input.ReadAsync();

var buffer = result.Buffer;
var consumed = buffer.Start;
var examined = buffer.End;
Expand All @@ -363,6 +368,7 @@ internal async Task<bool> HandshakeAsync(TimeSpan timeout, IReadOnlyList<string>
if (result.IsCanceled)
{
Log.HandshakeCanceled(_logger);
await WriteHandshakeResponseAsync(new HandshakeResponseMessage("Handshake was canceled."));
return false;
}

Expand Down Expand Up @@ -434,7 +440,7 @@ await WriteHandshakeResponseAsync(new HandshakeResponseMessage(
}
finally
{
_connectionContext.Transport.Input.AdvanceTo(consumed, examined);
input.AdvanceTo(consumed, examined);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.SignalR;
using Microsoft.AspNetCore.SignalR.Internal;
using Microsoft.Extensions.DependencyInjection.Extensions;
Expand Down Expand Up @@ -36,6 +37,8 @@ public static ISignalRServerBuilder AddHubOptions<THub>(this ISignalRServerBuild
public static ISignalRServerBuilder AddSignalR(this IServiceCollection services)
{
services.AddConnections();
// Disable the WebSocket keep alive since SignalR has it's own
services.Configure<WebSocketOptions>(o => o.KeepAliveInterval = TimeSpan.Zero);
services.TryAddSingleton<SignalRMarkerService>();
services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<HubOptions>, HubOptionsSetup>());
return services.AddSignalRCore();
Expand Down

0 comments on commit defbadb

Please sign in to comment.