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

chttp refactor #211

Merged
merged 16 commits into from Jan 29, 2018

Conversation

Projects
None yet
3 participants
@tanner0101
Copy link
Member

tanner0101 commented Jan 25, 2018

  • only copy headers once per TCP buffer
  • use static HTTP body if it is contained entirely in one TCP buffer (streamed otherwise)
  • refactor CParserResults and other CHTTPParser code into a single CHTTPParserContext

Seems to yield a solid ~20% speedup (vs 110k I was seeing previously).

Running 1s test @ http://localhost:8123
  4 threads and 128 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.36ms    2.47ms  45.41ms   94.89%
    Req/Sec    33.03k     2.93k   39.92k    75.00%
  144548 requests in 1.10s, 5.24MB read
Requests/sec: 131320.48
Transfer/sec:      4.76MB

tanner0101 added some commits Jan 23, 2018

@tanner0101 tanner0101 added this to the 3.0.0 milestone Jan 25, 2018

@tanner0101 tanner0101 self-assigned this Jan 25, 2018

tanner0101 added some commits Jan 25, 2018

@tanner0101

This comment has been minimized.

Copy link
Member Author

tanner0101 commented Jan 26, 2018

@tanner0101

This comment has been minimized.

Copy link
Member Author

tanner0101 commented Jan 26, 2018

Got another 10k out of it.

Running 1s test @ http://localhost:8123
  4 threads and 128 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     0.90ms   39.82us   1.92ms   84.22%
    Req/Sec    35.29k   552.40    35.99k    80.00%
  140363 requests in 1.00s, 5.09MB read
Requests/sec: 140194.07
Transfer/sec:      5.08MB
@twof

This comment has been minimized.

Copy link
Member

twof commented Jan 26, 2018

What are your hardware specs? @Joannis was topping out at 57k last I heard on a Macbook Pro

@Joannis

This comment has been minimized.

Copy link
Member

Joannis commented Jan 26, 2018

@twof that was using Vapor 3 (not the HTTP Engine). The framework has a bit more overhead, and the situation I described was a lot more complex than a static response. It was using a few DB queries, and a larger (10 KB I think) JSON blob.

The 57K was argued as a "I didn't manage to drop below this point).

I ran the above tests with the following results:

Running 10s test @ http://localhost:8123
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     0.88ms  279.03us   8.01ms   84.40%
    Req/Sec    27.98k     2.45k   40.85k    82.75%
  1113630 requests in 10.00s, 142.31MB read
Requests/sec: 111324.52
Transfer/sec:     14.23MB

For comparison, FastHTTP, which is a really fast Go webserver has these results for me on my macbook pro:

Running 10s test @ http://localhost:8080
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   834.70us  209.37us   8.09ms   95.43%
    Req/Sec    29.77k     1.93k   32.92k    84.25%
  1185007 requests in 10.00s, 169.52MB read
Requests/sec: 118479.28
Transfer/sec:     16.95MB

FastHTTP is adding a content-type header that my test is not, so that puts this test slightly out of balance. I am surprised by our consistency of the performance. It didn't change more than 0.01ms in my test runs

EDIT:
If we set the socket source and sink maxExcessSignalCount to 3, rather than 2, the performance goes up quite a bit. Also; I updated with the highest results for FastHTTP and Vapor 3 Engine.

/// The underlying storage type
var storage: Storage

var storage: HTTPBodyStorage

This comment has been minimized.

@Joannis

Joannis Jan 26, 2018

Member

A lot cleaner

private var indexes: [HTTPHeaderIndex?]

/// Creates a new `HTTPHeaders` with default content.
public static func `default`() -> HTTPHeaderStorage {

This comment has been minimized.

@Joannis

Joannis Jan 26, 2018

Member

This cannot be public, because the type is internal

}

extension UnsafeBufferPointer {
var start: UnsafePointer<Element> {

This comment has been minimized.

@Joannis

Joannis Jan 26, 2018

Member

These would be nice in bits

internal mutating func updateBodyHeaders() {
if let count = body.count {
if count > 0 {
headers[.contentLength] = count.description

This comment has been minimized.

@Joannis

This comment has been minimized.

@tanner0101

tanner0101 Jan 27, 2018

Author Member

That one is tough because currently the appendValue/removeValue API is kept private (which is great if possible because it minimizes risk of misuse).

tanner0101 added some commits Jan 27, 2018

@tanner0101 tanner0101 merged commit 085615e into beta Jan 29, 2018

2 checks passed

ci/circleci: linux Your tests passed on CircleCI!
Details
ci/circleci: macos Your tests passed on CircleCI!
Details

@tanner0101 tanner0101 added this to Done in Vapor 3 Feb 12, 2018

@tanner0101 tanner0101 deleted the chttp-refactor branch Feb 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment