Skip to content

Fix some low hanging allocation fruit #31115

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.

Sign up for GitHub

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 2 commits into from
Mar 22, 2021
Merged

Fix some low hanging allocation fruit #31115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08fc87a
Fix some low hanging allocation fruit
davidfowl Mar 21, 2021
7fcaa5c
No longer nullable
davidfowl Mar 21, 2021
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
4 changes: 2 additions & 2 deletions src/Middleware/WebSockets/src/HandshakeHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal static class HandshakeHelpers
/// <summary>
/// Gets request headers needed process the handshake on the server.
/// </summary>
public static readonly IEnumerable<string> NeededHeaders = new[]
public static readonly string[] NeededHeaders = new[]
Copy link
Member Author

Choose a reason for hiding this comment

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

PS: We should never do this. It's just inefficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

PS: We should never do this. It's just inefficient and prevents the JIT from optimizing.

Copy link
Member

Choose a reason for hiding this comment

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

+1

I guess it was done to prevent the array from ever being modified.

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'm gonna have to find all of the code that does this and fix it. Allocation profiles show interesting things. This also tends to happen when we resolve IEnumerable<T> from the DI container. Thought that one is harder to generalize, in hot code paths, it might be worth try-casting to T[] and falling back to IEnumerable<T> or IList<T> if used many times.

{
HeaderNames.Upgrade,
HeaderNames.Connection,
Expand All @@ -34,7 +34,7 @@ internal static class HandshakeHelpers
};

// Verify Method, Upgrade, Connection, version, key, etc..
public static bool CheckSupportedWebSocketRequest(string method, IEnumerable<KeyValuePair<string, string>> headers)
public static bool CheckSupportedWebSocketRequest(string method, List<KeyValuePair<string, string>> headers)
{
bool validUpgrade = false, validConnection = false, validKey = false, validVersion = false;

Expand Down
74 changes: 37 additions & 37 deletions src/SignalR/SignalR.slnf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,72 +1,72 @@
{
{
"solution": {
"path": "..\\..\\AspNetCore.sln",
"projects" : [
"src\\SignalR\\samples\\SignalRSamples\\SignalRSamples.csproj",
"src\\SignalR\\samples\\SocialWeather\\SocialWeather.csproj",
"src\\SignalR\\samples\\ClientSample\\ClientSample.csproj",
"src\\SignalR\\samples\\WebSocketSample\\WebSocketSample.csproj",
"src\\SignalR\\samples\\JwtClientSample\\JwtClientSample.csproj",
"src\\SignalR\\clients\\ts\\FunctionalTests\\SignalR.Client.FunctionalTestApp.csproj",
"projects": [
"src\\DataProtection\\Abstractions\\src\\Microsoft.AspNetCore.DataProtection.Abstractions.csproj",
"src\\DataProtection\\Cryptography.Internal\\src\\Microsoft.AspNetCore.Cryptography.Internal.csproj",
"src\\DataProtection\\DataProtection\\src\\Microsoft.AspNetCore.DataProtection.csproj",
"src\\FileProviders\\Embedded\\src\\Microsoft.Extensions.FileProviders.Embedded.csproj",
"src\\Hosting\\Abstractions\\src\\Microsoft.AspNetCore.Hosting.Abstractions.csproj",
"src\\Hosting\\Hosting\\src\\Microsoft.AspNetCore.Hosting.csproj",
"src\\Hosting\\Server.Abstractions\\src\\Microsoft.AspNetCore.Hosting.Server.Abstractions.csproj",
"src\\Hosting\\Server.IntegrationTesting\\src\\Microsoft.AspNetCore.Server.IntegrationTesting.csproj",
"src\\Hosting\\TestHost\\src\\Microsoft.AspNetCore.TestHost.csproj",
"src\\Http\\Authentication.Abstractions\\src\\Microsoft.AspNetCore.Authentication.Abstractions.csproj",
"src\\Http\\Authentication.Core\\src\\Microsoft.AspNetCore.Authentication.Core.csproj",
"src\\Http\\Headers\\src\\Microsoft.Net.Http.Headers.csproj",
"src\\Http\\Http.Abstractions\\src\\Microsoft.AspNetCore.Http.Abstractions.csproj",
"src\\Http\\Http.Extensions\\src\\Microsoft.AspNetCore.Http.Extensions.csproj",
"src\\Http\\Http.Features\\src\\Microsoft.AspNetCore.Http.Features.csproj",
"src\\Http\\Http\\src\\Microsoft.AspNetCore.Http.csproj",
"src\\Http\\Metadata\\src\\Microsoft.AspNetCore.Metadata.csproj",
"src\\Http\\Routing.Abstractions\\src\\Microsoft.AspNetCore.Routing.Abstractions.csproj",
"src\\Http\\Routing\\src\\Microsoft.AspNetCore.Routing.csproj",
"src\\Http\\WebUtilities\\src\\Microsoft.AspNetCore.WebUtilities.csproj",
"src\\Middleware\\CORS\\src\\Microsoft.AspNetCore.Cors.csproj",
"src\\Middleware\\Diagnostics.Abstractions\\src\\Microsoft.AspNetCore.Diagnostics.Abstractions.csproj",
"src\\Middleware\\Diagnostics\\src\\Microsoft.AspNetCore.Diagnostics.csproj",
"src\\Middleware\\HttpOverrides\\src\\Microsoft.AspNetCore.HttpOverrides.csproj",
"src\\Middleware\\StaticFiles\\src\\Microsoft.AspNetCore.StaticFiles.csproj",
"src\\Middleware\\WebSockets\\src\\Microsoft.AspNetCore.WebSockets.csproj",
"src\\Security\\Authentication\\Cookies\\src\\Microsoft.AspNetCore.Authentication.Cookies.csproj",
"src\\Security\\Authentication\\Core\\src\\Microsoft.AspNetCore.Authentication.csproj",
"src\\Security\\Authentication\\JwtBearer\\src\\Microsoft.AspNetCore.Authentication.JwtBearer.csproj",
"src\\Security\\Authorization\\Core\\src\\Microsoft.AspNetCore.Authorization.csproj",
"src\\Security\\Authorization\\Policy\\src\\Microsoft.AspNetCore.Authorization.Policy.csproj",
"src\\Servers\\Connections.Abstractions\\src\\Microsoft.AspNetCore.Connections.Abstractions.csproj",
"src\\Servers\\IIS\\IISIntegration\\src\\Microsoft.AspNetCore.Server.IISIntegration.csproj",
"src\\Servers\\Kestrel\\Core\\src\\Microsoft.AspNetCore.Server.Kestrel.Core.csproj",
"src\\Servers\\Kestrel\\Kestrel\\src\\Microsoft.AspNetCore.Server.Kestrel.csproj",
"src\\SignalR\\samples\\JwtSample\\JwtSample.csproj",
"src\\Servers\\Kestrel\\Transport.Sockets\\src\\Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.csproj",
"src\\SignalR\\clients\\csharp\\Client.Core\\src\\Microsoft.AspNetCore.SignalR.Client.Core.csproj",
"src\\SignalR\\clients\\csharp\\Client\\src\\Microsoft.AspNetCore.SignalR.Client.csproj",
"src\\SignalR\\clients\\csharp\\Client\\test\\FunctionalTests\\Microsoft.AspNetCore.SignalR.Client.FunctionalTests.csproj",
"src\\SignalR\\clients\\csharp\\Client\\test\\UnitTests\\Microsoft.AspNetCore.SignalR.Client.Tests.csproj",
"src\\SignalR\\clients\\csharp\\Http.Connections.Client\\src\\Microsoft.AspNetCore.Http.Connections.Client.csproj",
"src\\SignalR\\clients\\ts\\FunctionalTests\\SignalR.Client.FunctionalTestApp.csproj",
"src\\SignalR\\common\\Http.Connections.Common\\src\\Microsoft.AspNetCore.Http.Connections.Common.csproj",
"src\\SignalR\\common\\Http.Connections\\src\\Microsoft.AspNetCore.Http.Connections.csproj",
"src\\SignalR\\common\\Http.Connections\\test\\Microsoft.AspNetCore.Http.Connections.Tests.csproj",
"src\\SignalR\\common\\Protocols.Json\\src\\Microsoft.AspNetCore.SignalR.Protocols.Json.csproj",
"src\\SignalR\\common\\Protocols.MessagePack\\src\\Microsoft.AspNetCore.SignalR.Protocols.MessagePack.csproj",
"src\\SignalR\\common\\Protocols.NewtonsoftJson\\src\\Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson.csproj",
"src\\SignalR\\common\\SignalR.Common\\src\\Microsoft.AspNetCore.SignalR.Common.csproj",
"src\\SignalR\\common\\SignalR.Common\\test\\Microsoft.AspNetCore.SignalR.Common.Tests.csproj",
"src\\SignalR\\common\\testassets\\Tests.Utils\\Microsoft.AspNetCore.SignalR.Tests.Utils.csproj",
"src\\SignalR\\perf\\Microbenchmarks\\Microsoft.AspNetCore.SignalR.Microbenchmarks.csproj",
"src\\SignalR\\samples\\ClientSample\\ClientSample.csproj",
"src\\SignalR\\samples\\JwtClientSample\\JwtClientSample.csproj",
"src\\SignalR\\samples\\JwtSample\\JwtSample.csproj",
"src\\SignalR\\samples\\SignalRSamples\\SignalRSamples.csproj",
"src\\SignalR\\samples\\SocialWeather\\SocialWeather.csproj",
"src\\SignalR\\samples\\WebSocketSample\\WebSocketSample.csproj",
"src\\SignalR\\server\\Core\\src\\Microsoft.AspNetCore.SignalR.Core.csproj",
"src\\SignalR\\server\\SignalR\\src\\Microsoft.AspNetCore.SignalR.csproj",
"src\\SignalR\\server\\SignalR\\test\\Microsoft.AspNetCore.SignalR.Tests.csproj",
"src\\SignalR\\common\\Protocols.NewtonsoftJson\\src\\Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson.csproj",
"src\\SignalR\\server\\Specification.Tests\\src\\Microsoft.AspNetCore.SignalR.Specification.Tests.csproj",
"src\\SignalR\\server\\StackExchangeRedis\\src\\Microsoft.AspNetCore.SignalR.StackExchangeRedis.csproj",
"src\\SignalR\\server\\StackExchangeRedis\\test\\Microsoft.AspNetCore.SignalR.StackExchangeRedis.Tests.csproj",
"src\\Hosting\\Abstractions\\src\\Microsoft.AspNetCore.Hosting.Abstractions.csproj",
"src\\Http\\Headers\\src\\Microsoft.Net.Http.Headers.csproj",
"src\\Servers\\Connections.Abstractions\\src\\Microsoft.AspNetCore.Connections.Abstractions.csproj",
"src\\Http\\Http.Features\\src\\Microsoft.AspNetCore.Http.Features.csproj",
"src\\Http\\Http.Extensions\\src\\Microsoft.AspNetCore.Http.Extensions.csproj",
"src\\Security\\Authorization\\Core\\src\\Microsoft.AspNetCore.Authorization.csproj",
"src\\Hosting\\Server.Abstractions\\src\\Microsoft.AspNetCore.Hosting.Server.Abstractions.csproj",
"src\\Http\\Authentication.Abstractions\\src\\Microsoft.AspNetCore.Authentication.Abstractions.csproj",
"src\\Http\\WebUtilities\\src\\Microsoft.AspNetCore.WebUtilities.csproj",
"src\\DataProtection\\DataProtection\\src\\Microsoft.AspNetCore.DataProtection.csproj",
"src\\Middleware\\WebSockets\\src\\Microsoft.AspNetCore.WebSockets.csproj",
"src\\Middleware\\Diagnostics.Abstractions\\src\\Microsoft.AspNetCore.Diagnostics.Abstractions.csproj",
"src\\DataProtection\\Cryptography.Internal\\src\\Microsoft.AspNetCore.Cryptography.Internal.csproj",
"src\\DataProtection\\Abstractions\\src\\Microsoft.AspNetCore.DataProtection.Abstractions.csproj",
"src\\Servers\\Kestrel\\Core\\src\\Microsoft.AspNetCore.Server.Kestrel.Core.csproj",
"src\\Servers\\Kestrel\\Transport.Sockets\\src\\Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.csproj",
"src\\Http\\Routing\\src\\Microsoft.AspNetCore.Routing.csproj",
"src\\Http\\Routing.Abstractions\\src\\Microsoft.AspNetCore.Routing.Abstractions.csproj",
"src\\Security\\Authorization\\Policy\\src\\Microsoft.AspNetCore.Authorization.Policy.csproj",
"src\\SignalR\\clients\\csharp\\Client\\src\\Microsoft.AspNetCore.SignalR.Client.csproj",
"src\\SignalR\\clients\\csharp\\Client.Core\\src\\Microsoft.AspNetCore.SignalR.Client.Core.csproj",
"src\\Middleware\\HttpOverrides\\src\\Microsoft.AspNetCore.HttpOverrides.csproj",
"src\\SignalR\\common\\Protocols.Json\\src\\Microsoft.AspNetCore.SignalR.Protocols.Json.csproj",
"src\\Http\\Metadata\\src\\Microsoft.AspNetCore.Metadata.csproj",
"src\\WebEncoders\\src\\Microsoft.Extensions.WebEncoders.csproj",
"src\\FileProviders\\Embedded\\src\\Microsoft.Extensions.FileProviders.Embedded.csproj"
"src\\WebEncoders\\src\\Microsoft.Extensions.WebEncoders.csproj"
]
}
}
}
11 changes: 11 additions & 0 deletions src/SignalR/common/Http.Connections/src/HttpConnectionDispatcherOptions.cs
View file
Open in desktop
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.Collections.Generic;
using System.IO.Pipelines;
using Microsoft.AspNetCore.Authorization;

namespace Microsoft.AspNetCore.Http.Connections
Expand All @@ -15,6 +16,9 @@ public class HttpConnectionDispatcherOptions
// There maybe the opportunity for performance gains by tuning this default.
private const int DefaultPipeBufferSize = 32768;

private PipeOptions? _transportPipeOptions;
private PipeOptions? _appPipeOptions;

/// <summary>
/// Initializes a new instance of the <see cref="HttpConnectionDispatcherOptions"/> class.
/// </summary>
Expand Down Expand Up @@ -63,5 +67,12 @@ public HttpConnectionDispatcherOptions()
/// The default value is 0, the lowest possible protocol version.
/// </summary>
public int MinimumProtocolVersion { get; set; } = 0;

// We initialize these lazily based on the state of the options specified here.
// Though these are mutable it's extremely rare that they would be mutated past the
// call to initialize the routerware.
internal PipeOptions TransportPipeOptions => _transportPipeOptions ??= new PipeOptions(pauseWriterThreshold: TransportMaxBufferSize, resumeWriterThreshold: TransportMaxBufferSize / 2, readerScheduler: PipeScheduler.ThreadPool, useSynchronizationContext: false);

internal PipeOptions AppPipeOptions => _appPipeOptions ??= new PipeOptions(pauseWriterThreshold: ApplicationMaxBufferSize, resumeWriterThreshold: ApplicationMaxBufferSize / 2, readerScheduler: PipeScheduler.ThreadPool, useSynchronizationContext: false);
}
}
2 changes: 1 addition & 1 deletion src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ private async Task ExecuteApplication(ConnectionDelegate connectionDelegate)

// Jump onto the thread pool thread so blocking user code doesn't block the setup of the
// connection and transport
await AwaitableThreadPool.Yield();
await Task.Yield();
Copy link
Member Author

@davidfowl davidfowl Mar 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This should be equivalent in the default case as there's no sync context or custom task scheduler. The end result is that it won't allocate an additional task as it re-queues the state machine to the thread pool.


// Running this in an async method turns sync exceptions into async ones
await connectionDelegate(this);
Expand Down
5 changes: 3 additions & 2 deletions src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,8 +718,9 @@ private static void CloneHttpContext(HttpContext context, HttpConnectionContext

private HttpConnectionContext CreateConnection(HttpConnectionDispatcherOptions options, int clientProtocolVersion = 0)
{
var transportPipeOptions = new PipeOptions(pauseWriterThreshold: options.TransportMaxBufferSize, resumeWriterThreshold: options.TransportMaxBufferSize / 2, readerScheduler: PipeScheduler.ThreadPool, useSynchronizationContext: false);
var appPipeOptions = new PipeOptions(pauseWriterThreshold: options.ApplicationMaxBufferSize, resumeWriterThreshold: options.ApplicationMaxBufferSize / 2, readerScheduler: PipeScheduler.ThreadPool, useSynchronizationContext: false);
var transportPipeOptions = options.TransportPipeOptions;
var appPipeOptions = options.AppPipeOptions;

return _manager.CreateConnection(transportPipeOptions, appPipeOptions, clientProtocolVersion);
}

Expand Down