Skip to content

Managed RIO Server #12

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.

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 9 commits into from

Conversation

benaadams
Copy link
Contributor

Updated commit for #10

@DamianEdwards
Copy link
Member

Latest results below. Seems CPU use went up a bit and we lost peak RPS (down from >7m) as a result.

Connections Pipeline Depth Bandwidth RPS CPU
1 1 25 Mbps ~14k 13%
8 16 1.1 Gbps ~855k 37%
8 512 7.3 Gbps ~5.4m 40%
32 16 2.9 Gbps ~2.2m 80%
128 16 6.0 Gbps ~4.5m 100%
256 16 6.8 Gbps ~5.1m 100%
512 16 6.9 Gbps ~5.2m 100%
512 150 8.8 Gbps ~6.7m 84%
512 512 9.1 Gbps ~6.9m 67%

@Eilon Eilon closed this Jul 22, 2015
@Eilon Eilon reopened this Jul 22, 2015
@dnfclas
Copy link

dnfclas commented Jul 22, 2015

Hi @benaadams, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@Eilon
Copy link
Contributor

Eilon commented Jul 22, 2015

@DamianEdwards CLA bot is happy.

@benaadams
Copy link
Contributor Author

Hopefully this will max network before CPU

@DamianEdwards
Copy link
Member

Looking forward to trying this one 😄

@benaadams
Copy link
Contributor Author

Tried it on an Azure G5; need to put some SIMD fixes in as on that hardware Vector.Count operates on 32 bytes at the same time rather than 16 bytes (AVX 256 rather than SSE 128)

@benaadams
Copy link
Contributor Author

wrk -c 512 --latency -t 32 http://url/ -s ./scripts/pipeline.lua -- 148

Still a bit funny on NUMA - good but not great, probably need to do thread affinities there; but on Azure G4: 9.0 Gbps at 59% CPU:

azure-g4-windows

At 6.8M requests per second (6,807,542). Latency of 92.ms (avg), 113.81 ms (std-dev), 1.27s (max), 50% < 43ms, 90% < 288ms.

azure-g4-linux

@Bobris
Copy link

Bobris commented Jul 28, 2015

You do all crazy stuff with AVX and then use this in performance critical code???

                var date = DateTime.UtcNow.ToString("r");
                Encoding.UTF8.GetBytes(date, 0, dateBytes.Length, dateBytes, 0);

@benaadams
Copy link
Contributor Author

You do all crazy stuff with AVX and then use this in performance critical code???

😉

Lazy fix, function could be faster, but still should cut down the string allocs by 6.9M per second at 9.1 Gbps, to 512 per second; so not a bad change...

Takes my test on 68% CPU to 61%, so seems effective.

@benaadams benaadams force-pushed the managed-rio-experimental branch 2 times, most recently from 93d7f70 to d56a876 Compare July 29, 2015 00:18
@Bobris
Copy link

Bobris commented Jul 29, 2015

I think you would found DateTime.UtcNow slow too (it is basically Kernel call). This is how simply I do it in Nowin:
https://github.com/Bobris/Nowin/blob/master/Nowin/DateHeaderValueProvider.cs

@benaadams
Copy link
Contributor Author

This is how simply I do it in Nowin

That's much better than the convoluted improvement I was planning; gone for something similar.

@benaadams benaadams force-pushed the managed-rio-experimental branch from 1ac49d9 to cfe0190 Compare July 29, 2015 17:09
@benaadams benaadams force-pushed the managed-rio-experimental branch from c020670 to 0e0e695 Compare July 29, 2015 17:25
Assuming decent thread scheduling and allocation; move header and body data to same NUMA node
Shave a byte off per message by removing optional whitespace (or 5 bytes if you include the ones already removed)

https://tools.ietf.org/html/rfc7230#section-3.2.4

> A field value might be preceded and/or followed by optional
   whitespace (OWS); a single SP preceding the field-value is preferred
   for consistent readability by humans.
@RichiCoder1
Copy link

No dev comment, just wanted to say awesome job @benaadams 👍

@CIPop
Copy link

CIPop commented Aug 27, 2015

This is really cool.
@benaadams it would be very interesting to see how this compares with a native-only implementation of RIO: http://ctstraffic.codeplex.com/

@benaadams
Copy link
Contributor Author

@CIPop A reference implementation!

Super useful to know; I couldn't really find anything other than the api docs building this so its mostly trial and error iteration; will definitely have a look at it; thank you 👍

@CIPop
Copy link

CIPop commented Aug 28, 2015

Glad to help :)
I just learned about this yesterday. I'll update the Registered I/O Extensions tracking issue in CoreFX as well.

Keep alive doesn't need to be returned in headers for http/1.1
Revert buffer changes
@davidfowl
Copy link
Member

Closing this out

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

Successfully merging this pull request may close these issues.

8 participants