Skip to content

Add SkipLocalsInit #26598

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

Closed
wants to merge 2 commits into from
Closed

Add SkipLocalsInit #26598

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
ac10f7d
Add SkipLocalsInit
JamesNK Oct 5, 2020
54e0d40
Fix build
JamesNK Dec 1, 2020
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
3 changes: 3 additions & 0 deletions src/Hosting/Abstractions/src/Microsoft.AspNetCore.Hosting.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Description>ASP.NET Core hosting and startup abstractions for web applications.</Description>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore;hosting</PackageTags>
<IsPackable>false</IsPackable>
Expand All @@ -14,6 +15,8 @@
<Reference Include="Microsoft.AspNetCore.Hosting.Server.Abstractions" />
<Reference Include="Microsoft.AspNetCore.Http.Abstractions" />
<Reference Include="Microsoft.Extensions.Hosting.Abstractions" />

<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
Copy link
Member

Choose a reason for hiding this comment

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

Hosting.Abstractions is not on a perf-critical code path and from your review it doesn't contain any stackallocs. Why did you mark this assembly? The same goes for Server.Abstractions and likely some of the others.

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 added it to all projects referenced by Kestrel.Core. Want them removed?

Copy link
Member

Choose a reason for hiding this comment

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

What does it get us if we're not using stackallocs in these assemblies?

Copy link
Member Author

Choose a reason for hiding this comment

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

@stephentoub Is there much benefit to SkipLocalsInit outside of stackalloc usages?

Copy link
Member

@stephentoub stephentoub Oct 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The benefit isn't limited to stackalloc, but is proportional to how much state is on the stack that needs to be cleared (and where clearing can be skipped). As such, it generally is much more impactful with stackalloc, but it can show up as a peanut butter cost elsewhere.

[SkipLocalsInit]
[Benchmark]
public unsafe int WithSkipLocal()
{
    Guid g1, g2, g3, g4, g5, g6;

    // Do something cheap that let's the benchmark compile and stuff not get optimized away
    Unsafe.SkipInit(out g1);
    Unsafe.SkipInit(out g2);
    Unsafe.SkipInit(out g3);
    Unsafe.SkipInit(out g4);
    Unsafe.SkipInit(out g5);
    Unsafe.SkipInit(out g6);
    return *(int*)&g1 + *(int*)&g2 + *(int*)&g3 + *(int*)&g4 + *(int*)&g5 + *(int*)&g6;
}

[Benchmark]
public unsafe int WithoutSkipLocal()
{
    Guid g1, g2, g3, g4, g5, g6;

    // Do something cheap that let's the benchmark compile and stuff not get optimized away
    Unsafe.SkipInit(out g1);
    Unsafe.SkipInit(out g2);
    Unsafe.SkipInit(out g3);
    Unsafe.SkipInit(out g4);
    Unsafe.SkipInit(out g5);
    Unsafe.SkipInit(out g6);
    return *(int*)&g1 + *(int*)&g2 + *(int*)&g3 + *(int*)&g4 + *(int*)&g5 + *(int*)&g6;
}
Method Mean Error StdDev Code Size
WithSkipLocal 0.2983 ns 0.0324 ns 0.0303 ns 33 B
WithoutSkipLocal 0.7273 ns 0.0015 ns 0.0013 ns 79 B

If the goal was biggest bang for the buck, though, it's definitely where larger stackallocs are being employed.

Copy link
Member

@stephentoub stephentoub Oct 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Another place it can show up impactfully is with fixed-size buffers, since as with stackallocs, they can often represent large regions of stack space that need to be cleared, e.g.

[SkipLocalsInit]
[Benchmark]
public unsafe int WithSkipLocal()
{
    Data d;
    Unsafe.SkipInit(out d);
    return d.Ptr[0];
}

[Benchmark]
public unsafe int WithoutSkipLocal()
{
    Data d;
    Unsafe.SkipInit(out d);
    return d.Ptr[0];
}

unsafe struct Data
{
    public fixed byte Ptr[1024];
}
Method Mean Error StdDev Code Size
WithSkipLocal 0.5009 ns 0.0047 ns 0.0041 ns 64 B
WithoutSkipLocal 14.8256 ns 0.0411 ns 0.0343 ns 132 B

Copy link
Member

@Tratcher Tratcher Dec 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Given it comes with a degree of risk, it seems better not to add it unless here's a demonstrated benefit. I also assume you'd be targeting the hot paths like per-request processing. Hosting doesn't have many of those.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @Tratcher but we also don't clear the array from the array pool so I don't see how this is different. Still I'd prefer doing this to some key placed in Kestrel rather than the entire assembly. That'll build more confidence.

</ItemGroup>

</Project>
2 changes: 2 additions & 0 deletions src/Hosting/Hosting/src/Microsoft.AspNetCore.Hosting.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Description>ASP.NET Core hosting infrastructure and startup logic for web applications.</Description>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore;hosting</PackageTags>
<IsPackable>false</IsPackable>
Expand Down Expand Up @@ -31,6 +32,7 @@
<Reference Include="Microsoft.Extensions.Options" />

<Compile Include="$(SharedSourceRoot)TypeNameHelper\*.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

</Project>
3 changes: 3 additions & 0 deletions src/Hosting/Server.Abstractions/src/Microsoft.AspNetCore.Hosting.Server.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Description>ASP.NET Core hosting server abstractions for web applications.</Description>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore;hosting</PackageTags>
<IsPackable>false</IsPackable>
Expand All @@ -13,6 +14,8 @@
<ItemGroup>
<Reference Include="Microsoft.AspNetCore.Http.Features" />
<Reference Include="Microsoft.Extensions.Configuration.Abstractions" />

<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

</Project>
4 changes: 4 additions & 0 deletions src/Http/Headers/src/Microsoft.Net.Http.Headers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,8 @@
<Reference Include="Microsoft.Extensions.Primitives" />
</ItemGroup>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

</Project>
2 changes: 2 additions & 0 deletions src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Microsoft.AspNetCore.Http.HttpResponse</Description>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore</PackageTags>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsPackable>false</IsPackable>
<Nullable>enable</Nullable>
</PropertyGroup>
Expand All @@ -23,6 +24,7 @@ Microsoft.AspNetCore.Http.HttpResponse</Description>
<Compile Include="$(SharedSourceRoot)ActivatorUtilities\*.cs" />
<Compile Include="$(SharedSourceRoot)ParameterDefaultValue\*.cs" />
<Compile Include="$(SharedSourceRoot)PropertyHelper\**\*.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<NoWarn>$(NoWarn);CS1591</NoWarn>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore</PackageTags>
<IsPackable>false</IsPackable>
Expand All @@ -13,6 +14,7 @@

<ItemGroup>
<Compile Include="..\..\Shared\StreamCopyOperationInternal.cs" Link="StreamCopyOperationInternal.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions src/Http/Http.Features/src/Microsoft.AspNetCore.Http.Features.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<TargetFrameworks Condition="'$(DotNetBuildFromSource)' == 'true'">$(DefaultNetCoreTargetFramework)</TargetFrameworks>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<NoWarn>$(NoWarn);CS1591</NoWarn>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore</PackageTags>
<Nullable>enable</Nullable>
Expand All @@ -15,6 +16,8 @@
<ItemGroup>
<Reference Include="Microsoft.Extensions.Primitives" />
<Reference Include="System.IO.Pipelines" />

<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

</Project>
1 change: 1 addition & 0 deletions src/Http/Http/src/Microsoft.AspNetCore.Http.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<Compile Include="$(SharedSourceRoot)ValueTaskExtensions\**\*.cs" />
<Compile Include="..\..\Shared\StreamCopyOperationInternal.cs" Link="Internal\StreamCopyOperationInternal.cs" />
<Compile Include="..\..\WebUtilities\src\AspNetCoreTempDirectory.cs" LinkBase="Internal" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/Http/Routing.Abstractions/src/Microsoft.AspNetCore.Routing.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ Microsoft.AspNetCore.Routing.RouteData</Description>
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore;routing</PackageTags>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsPackable>false</IsPackable>
<Nullable>annotations</Nullable>
</PropertyGroup>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)PropertyHelper\*.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
1 change: 1 addition & 0 deletions src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Microsoft.AspNetCore.Routing.RouteCollection</Description>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)PropertyHelper\*.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<DefineConstants>$(DefineConstants);WebEncoders_In_WebUtilities</DefineConstants>
<NoWarn>$(NoWarn);CS1591</NoWarn>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore</PackageTags>
<IsPackable>false</IsPackable>
Expand All @@ -15,6 +16,7 @@
<ItemGroup>
<Compile Include="$(SharedSourceRoot)WebEncoders\**\*.cs" />
<Compile Include="$(SharedSourceRoot)UrlDecoder\**\*.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 2 additions & 0 deletions ...Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore</PackageTags>
<NoWarn>CS1591;$(NoWarn)</NoWarn>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>enable</Nullable>
</PropertyGroup>

Expand All @@ -17,6 +18,7 @@
<Compile Include="$(SharedSourceRoot)ActivatorUtilities\*.cs" />
<Compile Include="$(SharedSourceRoot)ParameterDefaultValue\*.cs" />
<Compile Include="$(SharedSourceRoot)CodeAnalysis\*.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0' OR '$(TargetFramework)' == '$(DefaultNetFxTargetFramework)'">
Expand Down
2 changes: 2 additions & 0 deletions src/Servers/Kestrel/Core/src/Microsoft.AspNetCore.Server.Kestrel.Core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
<Compile Include="$(SharedSourceRoot)Hpack\**\*.cs" Link="Shared\Hpack\%(Filename)%(Extension)" />
<Compile Include="$(SharedSourceRoot)ServerInfrastructure\**\*.cs" />
<Compile Include="$(RepoRoot)src\Shared\TaskToApm.cs" Link="Internal\TaskToApm.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />

</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions src/Servers/Kestrel/Kestrel/src/Microsoft.AspNetCore.Server.Kestrel.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<IsAspNetCoreApp>true</IsAspNetCoreApp>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore;kestrel</PackageTags>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsPackable>false</IsPackable>
<Nullable>enable</Nullable>
</PropertyGroup>
Expand All @@ -14,6 +15,8 @@
<Reference Include="Microsoft.AspNetCore.Hosting" />
<Reference Include="Microsoft.AspNetCore.Server.Kestrel.Core" />
<Reference Include="Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets" />

<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

</Project>
1 change: 1 addition & 0 deletions ...rs/Kestrel/Transport.Libuv/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<Compile Include="$(KestrelSharedSourceRoot)\TransportConnection.cs" Link="Internal\TransportConnection.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\TransportConnection.Generated.cs" Link="Internal\TransportConnection.Generated.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\TransportConnection.FeatureCollection.cs" Link="Internal\TransportConnection.FeatureCollection.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
1 change: 1 addition & 0 deletions ...Transport.Quic/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Experimental.Quic.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
<Compile Include="$(KestrelSharedSourceRoot)\TransportMultiplexedConnection.Generated.cs" Link="Internal\TransportMultiplexedConnection.Generated.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\TransportMultiplexedConnection.FeatureCollection.cs" Link="Internal\TransportMultiplexedConnection.FeatureCollection.cs" />
<Compile Include="$(RepoRoot)src\Shared\TaskToApm.cs" Link="Internal\TaskToApm.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 2 additions & 1 deletion ...estrel/Transport.Sockets/src/Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Description>Managed socket transport for the ASP.NET Core Kestrel cross-platform web server.</Description>
Expand All @@ -18,6 +18,7 @@
<Compile Include="$(KestrelSharedSourceRoot)\TransportConnection.cs" Link="Internal\TransportConnection.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\TransportConnection.Generated.cs" Link="Internal\TransportConnection.Generated.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\TransportConnection.FeatureCollection.cs" Link="Internal\TransportConnection.FeatureCollection.cs" />
<Compile Include="$(SharedSourceRoot)SkipLocalsInit.cs" LinkBase="Shared\SkipLocalsInit.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
15 changes: 15 additions & 0 deletions src/Shared/SkipLocalsInit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

// Used to indicate to the compiler that the .locals init flag should not be set in method headers.
#if NET6_0
[module: System.Runtime.CompilerServices.SkipLocalsInit]
#elif NETSTANDARD2_0
// Not available
#elif NETSTANDARD2_1
// Not available
#elif NET461
// Not available
#else
#error Target frameworks need to be updated.
#endif