Skip to content
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

HTTP/1 headers simplification & cleanup #857

Merged
merged 1 commit into from Mar 6, 2019

Conversation

weissi
Copy link
Member

@weissi weissi commented Feb 28, 2019

Motivation:

The HTTP/1 headers were quite complicated, CoW-boxed and exposed their
guts (being a ByteBuffer). In Swift 5 this is no longer necessary
because of native UTF-8 Strings.

Modifications:

  • make headers simply [(String, String)]
  • remove HTTPHeaderIndex, HTTPListHeaderIterator, etc

Result:

  • simpler and more performant code
  • nice speedup:

before:

$ wrk http://localhost:8888
Running 10s test @ http://localhost:8888
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   162.88us   44.81us   1.79ms   75.12%
    Req/Sec    28.75k     2.22k   32.83k    80.20%
  577919 requests in 10.10s, 28.66MB read
Requests/sec:  57218.56
Transfer/sec:      2.84MB
$ tcpkali -em "GET / HTTP/1.1\r\nHost: google.com\r\n\r\n" -r 10000000000000 --latency-marker "HTTP/1.1" 127.0.0.1:8888
Destination: [127.0.0.1]:8888
Interface lo0 address [127.0.0.1]:0
Using interface lo0 to connect to [127.0.0.1]:8888
Ramped up to 1 connections.
Total data sent:     15.2 MiB (15916740 bytes)
Total data received: 20.8 MiB (21827312 bytes)
Bandwidth per channel: 30.174⇅ Mbps (3771.7 kBps)
Aggregate bandwidth: 17.449↓, 12.724↑ Mbps
Packet rate estimate: 40324.3↓, 1101.8↑ (1↓, 37↑ TCP MSS/op)
Message latency at percentiles: 672.7/691.5/695.5 ms (95/99/99.5%)
Test duration: 10.0071 s.

after:

$ wrk http://localhost:8888
Running 10s test @ http://localhost:8888
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   151.39us   47.03us   1.40ms   80.82%
    Req/Sec    30.72k     3.80k   53.58k    67.16%
  614191 requests in 10.10s, 30.46MB read
Requests/sec:  60814.41
Transfer/sec:      3.02MB
$ tcpkali -em "GET / HTTP/1.1\r\nHost: google.com\r\n\r\n" -r 10000000000000 --latency-marker "HTTP/1.1" 127.0.0.1:8888
Destination: [127.0.0.1]:8888
Interface lo0 address [127.0.0.1]:0
Using interface lo0 to connect to [127.0.0.1]:8888
Ramped up to 1 connections.
Total data sent:     18.1 MiB (18945684 bytes)
Total data received: 24.9 MiB (26158600 bytes)
Bandwidth per channel: 36.077⇅ Mbps (4509.6 kBps)
Aggregate bandwidth: 20.923↓, 15.154↑ Mbps
Packet rate estimate: 47387.0↓, 1316.9↑ (1↓, 37↑ TCP MSS/op)
Message latency at percentiles: 557.5/571.5/576.3 ms (95/99/99.5%)
Test duration: 10.0019 s.

@weissi weissi added this to the 2.0.0 milestone Feb 28, 2019
@weissi weissi added the ⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0 label Feb 28, 2019
@weissi weissi mentioned this pull request Feb 28, 2019
@weissi weissi force-pushed the jw-wip-http-headers-cleanup branch 2 times, most recently from 49c0d32 to b2e44c9 Compare March 6, 2019 14:37
@weissi weissi changed the title simplify & cleanup http headers HTTP/1 headers simplification & cleanup Mar 6, 2019
@weissi weissi requested a review from Lukasa March 6, 2019 14:37
@weissi weissi marked this pull request as ready for review March 6, 2019 14:37
@weissi weissi force-pushed the jw-wip-http-headers-cleanup branch from b2e44c9 to ec34714 Compare March 6, 2019 15:30
Sources/NIOHTTP1/HTTPTypes.swift Outdated Show resolved Hide resolved
Motivation:

The HTTP/1 headers were quite complicated, CoW-boxed and exposed their
guts (being a ByteBuffer). In Swift 5 this is no longer necessary
because of native UTF-8 Strings.

Modifications:

- make headers simply `[(String, String)]`
- remove `HTTPHeaderIndex`, `HTTPListHeaderIterator`, etc

Result:

- simpler and more performant code
@weissi weissi force-pushed the jw-wip-http-headers-cleanup branch from 04a96d3 to b25faa6 Compare March 6, 2019 17:59
@weissi weissi requested a review from Lukasa March 6, 2019 17:59
@weissi weissi merged commit 22e6cd3 into apple:master Mar 6, 2019
@weissi weissi deleted the jw-wip-http-headers-cleanup branch March 6, 2019 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants