Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Ported HttpParserBench to BenchmarkDotNet #2243

Merged
merged 1 commit into from
May 2, 2018

Conversation

KrzysztofCwalina
Copy link
Member

BenchmarkDotNet=v0.10.14.525-nightly, OS=Windows 10.0.16299.371 (1709/FallCreatorsUpdate/Redstone3)
Intel Core i7-6700K CPU 4.00GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
Frequency=3914067 Hz, Resolution=255.4887 ns, Timer=TSC
.NET Core SDK=2.2.0-preview1-007460
  [Host]     : .NET Core 2.2.0-preview1-26424-02 (CoreCLR 4.6.26423.05, CoreFX 4.6.26423.04), 64bit RyuJIT
  DefaultJob : .NET Core 2.2.0-preview1-26424-02 (CoreCLR 4.6.26423.05, CoreFX 4.6.26423.04), 64bit RyuJIT

Method Mean Error StdDev
RequestLine 136.0 ns 0.1786 ns 0.1583 ns
Headers 251.0 ns 1.2776 ns 1.1951 ns
FullRequest 327.8 ns 0.2701 ns 0.2526 ns

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM, but will wait with merge until I get the answers

}
}

struct RequestStruct : IHttpHeadersHandler, IHttpRequestLineHandler
Copy link
Member

Choose a reason for hiding this comment

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

@krwq this type is not used (and was not before the port), should we keep it or remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about it when I was porting the tests. I actually would like to add the struct option to these tests. But for now, we are just porting, and I will do it as a separate workitem.

[Benchmark]
public void RequestLine()
{
var request = new Request();
Copy link
Member

Choose a reason for hiding this comment

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

@KrzysztofCwalina do parsing methods have any side effects on Request? if not, maybe we should make it a field too?

Copy link
Member Author

Choose a reason for hiding this comment

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

The code with side effects is commented out. After removing the comments the request cannot be a filed. Also, the cost of creating the request object is not avoidable, so I think it's best include it in the cost.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants