Remove array allocation from multi-span header parsing by BrennanConroy · Pull Request #45044 · dotnet/aspnetcore (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Noticed this allocation while looking at a memory profile for CertAuth. My hypothesis is that the certificate/tls handshake is pushing the header bytes across a PipeSegment boundary which goes through the multi-span header parsing path, and that path would allocate a byte[] if the header crossed multiple spans. This change avoids the allocation by using stackalloc or ArrayPool<byte>.Shared.

The highlighted line is now gone from the benchmark 😃

Before:

Method	Mean	Error	StdDev	Median	Op/s	Gen 0	Gen 1	Gen 2	Allocated
Unicode	371.2 ns	11.94 ns	35.22 ns	356.3 ns	2,693,742.3	-	-	-	-
MultispanUnicodeHeader	573.8 ns	18.61 ns	54.87 ns	552.9 ns	1,742,893.2	-	-	-	48 B

After:

Method	Mean	Error	StdDev	Median	Op/s	Gen 0	Gen 1	Gen 2	Allocated
Unicode	351.9 ns	10.64 ns	31.03 ns	339.3 ns	2,841,974.0	-	-	-	-
MultispanUnicodeHeader - First Commit	553.0 ns	16.74 ns	48.82 ns	533.3 ns	1,808,160.4	-	-	-	-
MultispanUnicodeHeader - Second Commit	484.9 ns	11.75 ns	33.90 ns	471.2 ns	2,062,450.8	-	-	-	-

The Op/s difference is noise, especially since Unicode shouldn't hit the changed code path.

I was able to improve MultispanUnicodeHeader to 2m Op/s locally by inlining the PositionOfAny method, skipping the first span (since it was checked by the caller), and doing operations with the ReadOnlyMemory returned from ReadOnlySequence.TryGet instead of all the GetPosition APIs on ReadOnlySequence. The code is a bit yuckier though, so not sure it's worth it.

Edit: Cleaned it up a bit so it's not too bad

Choose a reason for hiding this comment

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

🚀

@@ -53,6 +69,16 @@ public void Unicode()
}
}

[Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount)]
public void MultispanUnicodeHeader()

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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