HttpResponseHeaders.CopyToFast move directly to next rather than if branching

[benaadams]

Can use BitOperations.TrailingZeroCount on the set bits to determine which is the next header; rather than testing each bit in sequence via fall-through, droping x37 if statements from the CopyToFast response header output method.

Second commit is pulling Content-Length to the first header in tests (as its a special long? outlier header)

from

case 5: // Header: "Accept-Ranges"
    if ((tempBits & 0x10L) != 0) // Test
    {
        tempBits ^= 0x10L;
        values = ref _headers._AcceptRanges;
        keyStart = 48;
        keyLength = 17;
        next = 6;
        break; // OutputHeader
    }
    goto case 6; // Fall-through

case 6: // Header: "Access-Control-Allow-Credentials"
    if ((tempBits & 0x20L) != 0) // Test
    {
        tempBits ^= 0x20L;
        values = ref _headers._AccessControlAllowCredentials;
        keyStart = 65;
        keyLength = 36;
        next = 7;
        break; // OutputHeader
    }
    goto case 7; // Fall-through

to

case 4: // Header: "Accept-Ranges"
    values = ref _headers._AcceptRanges;
    keyStart = 48;
    keyLength = 17;
    break; // OutputHeader

case 5: // Header: "Access-Control-Allow-Credentials"
    values = ref _headers._AccessControlAllowCredentials;
    keyStart = 65;
    keyLength = 36;
    break; // OutputHeader

735 bytes less IL

- // IL Code size 2325 (0x915)
+ // IL Code size 1590 (0x636)

871 bytes less asm

- ; Total bytes of asm code: 3910
+ ; Total bytes of asm code: 3039

Branch	Type	Mean	Op/s	Delta
main	TechEmpowerPlaintext	56.79 ns	17,609,767.6	-
PR	TechEmpowerPlaintext	53.53 ns	18,679,950.2	+6.1%
main	PlaintextChunked	61.89 ns	16,158,595.4	-
PR	PlaintextChunked	47.82 ns	20,912,547.5	+29.4%
main	PlaintextWithCookie	133.94 ns	7,466,115.3	-
PR	PlaintextWithCookie	126.26 ns	7,920,045.3	+6.1%
main	Plain(...)ookie [26]	163.29 ns	6,124,234.5	-
PR	Plain(...)ookie [26]	168.58 ns	5,932,007.3	-3.1%
main	LiveAspNet	359.96 ns	2,778,117.3	-
PR	LiveAspNet	354.01 ns	2,824,802.3	+1.6%
main	Common	766.48 ns	1,304,672.7	-
PR	Common	721.08 ns	1,386,812.9	+6.3%

[benaadams]

/azp run

[azure-pipelines]

Commenter does not have sufficient privileges for PR 32337 in repo dotnet/aspnetcore

[Tratcher]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[benaadams]

Looks like CI has its own issues

[JamesNK]

CopyToFast is used by HTTP/1.1, but HTTP/2 uses the generated Enumerator types when encoding HPack response headers.

Can this improvement be applied to how the generated Enumerator types decide next? Writing response headers in HTTP/2 is a hotspot in our current implementation. Multiplexing and HPack means that only one response can write headers at a time while other responses wait on a lock. Reducing the time in the lock is huge.

[benaadams]

Can this improvement be applied to how the generated Enumerator types decide next?

Yes, though would do it in follow up

[benaadams]

Rebased for merge conflict

[JamesNK]

Can this improvement be applied to how the generated Enumerator types decide next?

Yes, though would do it in follow up

#33352

[benaadams]

Rebased to retrigger glitchy CI

[JamesNK]

I've rerun the build a couple of times and it is consistently failing. The Windows build is timing out.

[BrennanConroy]

D:\workspace_work\1\s\eng\targets\CSharp.Common.targets(115,5): error MSB5021: Terminating the task executable "cmd" and its child processes because the build was canceled. [D:\workspace_work\1\s\src\Servers\Kestrel\perf\Microbenchmarks\Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks.csproj]
SUCCESS: The process with PID 5864 (child process of PID 8004) has been terminated.
SUCCESS: The process with PID 8004 (child process of PID 2648) has been terminated.

Microbenchmarks validation is likely hanging. Something changed to make a single run of one or more of the the benchmarks hang.

'InMemoryTransportBenchmark.Plaintext: Dry(Toolchain=InProcessNoEmitToolchain, IterationCount=1, LaunchCount=1, RunStrategy=ColdStart, UnrollFactor=1, WarmupCount=1), 0 runs' failed, reason: 'AllMeasurements'

[Tratcher]

In hung_dotnet.exe_2596_210608_031244.dmp I see a thread hung at ConsoleLifetime.OnProcessExit waiting for the host be be disposed.
https://github.com/dotnet/runtime/blob/6f7f30e118d6652a89dd7261e31d33ecd75f66fe/src/libraries/Microsoft.Extensions.Hosting/src/Internal/ConsoleLifetime.cs#L79-L86

The hung process has a base directory of D:\workspace_work\1\s\artifacts\bin\Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks\Release\net6.0\0

Main thread callstack:

 	ntdll.dll!NtWaitForMultipleObjects�()	Unknown
 	KERNELBASE.dll!WaitForMultipleObjectsEx()	Unknown
 	[Inline Frame] hostpolicy.dll!coreclr_t::shutdown(int *) Line 132	C++
 	hostpolicy.dll!run_app_for_context(const hostpolicy_context_t & context, int argc, const wchar_t * * argv) Line 264	C++
 	hostpolicy.dll!run_app(const int argc, const wchar_t * * argv) Line 284	C++
 	hostpolicy.dll!corehost_main(const int argc, const wchar_t * * argv) Line 430	C++
 	hostfxr.dll!execute_app(const std::wstring & impl_dll_dir, corehost_init_t * init, const int argc, const wchar_t * * argv) Line 146	C++
 	hostfxr.dll!`anonymous namespace'::read_config_and_execute(const std::wstring & host_command, const host_startup_info_t & host_info, const std::wstring & app_candidate, const std::unordered_map<enum known_options,std::vector<std::wstring,std::allocator<std::wstring>>,known_options_hash,std::equal_to<enum known_options>,std::allocator<std::pair<enum known_options const ,std::vector<std::wstring,std::allocator<std::wstring>>>>> & opts, int new_argc, const wchar_t * * new_argv, host_mode_t mode, wchar_t * out_buffer, int buffer_size, int * required_buffer_size) Line 520	C++
 	hostfxr.dll!fx_muxer_t::handle_exec_host_command(const std::wstring & host_command, const host_startup_info_t & host_info, const std::wstring & app_candidate, const std::unordered_map<enum known_options,std::vector<std::wstring,std::allocator<std::wstring>>,known_options_hash,std::equal_to<enum known_options>,std::allocator<std::pair<enum known_options const ,std::vector<std::wstring,std::allocator<std::wstring>>>>> & opts, int argc, const wchar_t * * argv, int argoff, host_mode_t mode, wchar_t * result_buffer, int buffer_size, int * required_buffer_size) Line 1001	C++
 	hostfxr.dll!fx_muxer_t::execute(const std::wstring host_command, const int argc, const wchar_t * * argv, const host_startup_info_t & host_info, wchar_t * result_buffer, int buffer_size, int * required_buffer_size) Line 566	C++
>	hostfxr.dll!hostfxr_main_startupinfo(const int argc, const wchar_t * * argv, const wchar_t * host_path, const wchar_t * dotnet_root, const wchar_t * app_path) Line 61	C++
 	dotnet.exe!exe_start(const int argc, const wchar_t * * argv) Line 236	C++
 	dotnet.exe!wmain(const int argc, const wchar_t * * argv) Line 305	C++
 	[Inline Frame] dotnet.exe!invoke_main() Line 90	C++
 	dotnet.exe!__scrt_common_main_seh() Line 288	C++
 	kernel32.dll!BaseThreadInitThunk�()	Unknown
 	ntdll.dll!RtlUserThreadStart�()	Unknown

InMemoryTransportBenchmark looks like the only benchmark that creates a host. It does dispose it in GlobalCleanup, but maybe that's not happening or its failing?

aspnetcore/src/Servers/Kestrel/perf/Microbenchmarks/InMemoryTransportBenchmark.cs

Lines 45 to 59 in cee22ed

	_host = new HostBuilder()
	.ConfigureWebHost(webHostBuilder =>
	{
	webHostBuilder
	// Prevent VS from attaching to hosting startup which could impact results
	.UseSetting("preventHostingStartup", "true")
	.UseKestrel()
	// Bind to a single non-HTTPS endpoint
	.UseUrls("http://127.0.0.1:5000")
	.Configure(app => app.UseMiddleware<PlaintextMiddleware>());
	})
	.ConfigureServices(services => services.AddSingleton<IConnectionListenerFactory>(transportFactory))
	.Build();
	_host.Start();

aspnetcore/src/Servers/Kestrel/perf/Microbenchmarks/InMemoryTransportBenchmark.cs

Lines 84 to 88 in cee22ed

	[GlobalCleanup]
	public void GlobalCleanup()
	{
	_host.Dispose();
	}

[Tratcher]

Rebased, dropped the submodule changes, and added a commit that should help avoid the hang. Looking to get this merged to avoid conflicts with #33776.

[BrennanConroy]

Any idea why the hang started showing up with this change?

[Tratcher]

Any idea why the hang started showing up with this change?

No, it' wasn't even in an affected benchmark.

[Tratcher]

Nevermind, I found it:

The benchmark is throwing an exception related to this change (Content-Length ordering changed), and then GlobalCleanup doesn't run.

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.AggregateException: One or more errors occurred. (Invalid response
Expected:
HTTP/1.1 200 OK
Date: Fri, 02 Mar 2018 18:37:05 GMT
Content-Type: text/plain
Server: Kestrel
Content-Length: 13

Hello, World!
Actual:
HTTP/1.1 200 OK
Content-Length: 13
Content-Type: text/plain
Date: Tue, 29 Jun 2021 22:01:42 GMT
Server: Kestrel

Hello, World!)
 ---> System.InvalidOperationException: Invalid response
Expected:
HTTP/1.1 200 OK
Date: Fri, 02 Mar 2018 18:37:05 GMT
Content-Type: text/plain
Server: Kestrel
Content-Length: 13

Hello, World!
Actual:
HTTP/1.1 200 OK
Content-Length: 13
Content-Type: text/plain
Date: Tue, 29 Jun 2021 22:01:42 GMT
Server: Kestrel

Hello, World!
   at Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks.InMemoryTransportBenchmark.ValidateResponseAsync(Byte[] request, String expectedResponse) in D:\github\AspNetCore\src\Servers\Kestrel\perf\Microbenchmarks\InMemoryTransportBenchmark.cs:line 79
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions) in System.Private.CoreLib.dll:token 0x6002df0+0x11
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken) in System.Private.CoreLib.dll:token 0x6002e0c+0x3f
   at Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks.InMemoryTransportBenchmark.GlobalSetupPlaintext() in D:\github\AspNetCore\src\Servers\Kestrel\perf\Microbenchmarks\InMemoryTransportBenchmark.cs:line 62
   at BenchmarkDotNet.Engines.EngineFactory.CreateReadyToRun(EngineParameters engineParameters) in BenchmarkDotNet.dll:token 0x60009a7+0x0
   at BenchmarkDotNet.Autogenerated.Runnable_1.Run(BenchmarkCase benchmarkCase, IHost host) in RefEmit_InMemoryManifestModule:token 0x600002a+0x26
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) in System.Private.CoreLib.dll:token 0x6004f5c+0x23
   at BenchmarkDotNet.Toolchains.InProcess.Emit.Implementation.RunnableProgram.Run(BenchmarkId benchmarkId, Assembly partitionAssembly, BenchmarkCase benchmarkCase, IHost host) in BenchmarkDotNet.dll:token 0x600030b+0x0

[JamesNK]

Is there a way to make benchmark errors easier to debug? A friendly error message would be much better than an unidentified freeze.

[BrennanConroy]

I'm not sure if there is, but in most cases it's easy enough to run the benchmark in VS and see the issue

[Tratcher]

Thanks @benaadams