Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Scope for improving Microsoft.AspNetCore.Server.Kestrel.Internal.Http.Frame.TakeStartLine(SocketInput) #1213

Closed
sivarv opened this issue Nov 15, 2016 · 3 comments
Assignees

Comments

@sivarv
Copy link

sivarv commented Nov 15, 2016

TakeStartLine() is one of the hot methods in TechEmPower PlainText and JSON benchmarks.
Jitted code of this method could be improved in the following way:

This method calls the following methods by passing structs by value

MemoryPoolIteratorExtensions:GetAsciiString(struct,struct)
MemoryPoolIteratorExtensions:GetKnownMethod(struct,byref)
MemoryPoolIteratorExtensions:GetKnownVersion(struct,byref)
MemoryPoolIteratorExtensions:GetUtf8String(struct,struct)

The struct param in question is MemoryPoolIterator, which is a struct with two fields: a ref and an int. On Amd64 a local copy of struct param is made and a reference to local copy is passed by reference. Since there are many such calls, at each call site struct params needs to be copied to local. This struct passing overhead could be avoided if caller TakeStartLine() passes struct params by reference.

@sivarv sivarv changed the title Scope for improving Microsoft::AspNetCore::Server::Kestrel::Internal::Http::Frame::TakeStartLine(SocketInput) Scope for improving Microsoft.AspNetCore.Server.Kestrel.Internal.Http.Frame.TakeStartLine(SocketInput) Nov 15, 2016
@benaadams
Copy link
Contributor

Currently these methods are extensions and C# doesn't allow the first parameter to be byref dotnet/roslyn#165 Interestingly VB.NET does.

GetKnownVersion and GetKnownMethod could probably be aggressively inlined and remain extensions (this would have same effect?) as they are only used in one place.

GetAsciiString and GetUtf8String could be changed to regular methods as they are fairly general?

I have a benchmark that gives consistent results so will measure.

/cc @halter73 @CesarBS @davidfowl

@benaadams
Copy link
Contributor

ref changes benaadams@d3ffc67

Pre

Method Mean StdDev Median Scaled
ParsePlaintext 843.2055 ns 12.1499 ns 838.2252 ns 1.00
ParsePipelinedPlaintext 624.8637 ns 9.5501 ns 622.0941 ns 0.74
ParseLiveAspNet 3,990.4027 ns 40.8259 ns 3,999.2743 ns 4.73
ParsePipelinedLiveAspNet 3,640.7250 ns 55.9658 ns 3,630.0658 ns 4.32
ParseUnicode 6,957.7864 ns 134.4421 ns 6,943.0541 ns 8.25
ParseUnicodePipelined 6,560.8592 ns 100.9833 ns 6,598.5639 ns 7.78

Post

Method Mean StdDev Median Scaled
ParsePlaintext 785.7661 ns 15.2615 ns 785.2350 ns 1.00
ParsePipelinedPlaintext 620.8864 ns 6.3832 ns 624.4951 ns 0.79
ParseLiveAspNet 3,911.6923 ns 111.3273 ns 3,887.8120 ns 4.98
ParsePipelinedLiveAspNet 3,566.2522 ns 49.6079 ns 3,582.8719 ns 4.54
ParseUnicode 6,791.3005 ns 158.4868 ns 6,767.7367 ns 8.65
ParseUnicodePipelined 6,391.5840 ns 83.6463 ns 6,399.2505 ns 8.14

@benaadams
Copy link
Contributor

Added commit to #1138

@muratg muratg added this to the 1.2.0 milestone Nov 21, 2016
@muratg muratg modified the milestones: 1.2.0, 2.0.0 Jan 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants