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

Enable Ember Server Connection Reuse #3603

Merged

Conversation

ChristopherDavenport
Copy link
Member

@ChristopherDavenport ChristopherDavenport commented Jul 25, 2020

  • Enables Server Connection Reuse For Ember
  • Add Similar Processing as is done in Client, with implicit keep-alive and date.

Performance Locally

Blaze

hey -z 30s  http://localhost:8080/hello

Summary:
  Total:	30.0007 secs
  Slowest:	0.5015 secs
  Fastest:	0.0001 secs
  Average:	0.0015 secs
  Requests/sec:	75735.8589
  
  Total data:	68549378 bytes
  Size/request:	68 bytes

Response time histogram:
  0.000 [1]	|
  0.050 [999948]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  0.100 [1]	|
  0.151 [0]	|
  0.201 [0]	|
  0.251 [0]	|
  0.301 [0]	|
  0.351 [0]	|
  0.401 [0]	|
  0.451 [0]	|
  0.502 [50]	|


Latency distribution:
  10% in 0.0003 secs
  25% in 0.0004 secs
  50% in 0.0005 secs
  75% in 0.0007 secs
  90% in 0.0011 secs
  95% in 0.0014 secs
  99% in 0.0039 secs

Details (average, fastest, slowest):
  DNS+dialup:	0.0000 secs, 0.0001 secs, 0.5015 secs
  DNS-lookup:	0.0000 secs, 0.0000 secs, 0.0030 secs
  req write:	0.0000 secs, 0.0000 secs, 0.0051 secs
  resp wait:	0.0014 secs, 0.0001 secs, 0.4993 secs
  resp read:	0.0001 secs, 0.0000 secs, 0.0089 secs

Status code distribution:
  [200]	1000000 responses

Ember

hey -z 30s  http://localhost:8080/hello

Summary:
  Total:	30.0409 secs
  Slowest:	0.0541 secs
  Fastest:	0.0001 secs
  Average:	0.0024 secs
  Requests/sec:	20693.2210
  
  Total data:	18754221 bytes
  Size/request:	30 bytes

Response time histogram:
  0.000 [1]	|
  0.006 [593700]	|■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■
  0.011 [73]	|
  0.016 [9]	|
  0.022 [6]	|
  0.027 [6]	|
  0.032 [8]	|
  0.038 [6]	|
  0.043 [1289]	|
  0.049 [26514]	|■■
  0.054 [32]	|


Latency distribution:
  10% in 0.0002 secs
  25% in 0.0002 secs
  50% in 0.0003 secs
  75% in 0.0006 secs
  90% in 0.0010 secs
  95% in 0.0018 secs
  99% in 0.0475 secs

Details (average, fastest, slowest):
  DNS+dialup:	0.0000 secs, 0.0001 secs, 0.0541 secs
  DNS-lookup:	0.0000 secs, 0.0000 secs, 0.0007 secs
  req write:	0.0000 secs, 0.0000 secs, 0.0030 secs
  resp wait:	0.0003 secs, 0.0001 secs, 0.0379 secs
  resp read:	0.0021 secs, 0.0000 secs, 0.0485 secs

Status code distribution:
  [200]	621644 responses

@@ -101,12 +121,28 @@ private[server] object ServerHelpers {
for {
socket <- connect.flatMap(upgradeSocket(_, tlsInfoOpt))
out <- Resource.liftF(
Stream
.eval(runApp(socket).attempt)
(Stream(false) ++ Stream(true).repeat)
Copy link
Member Author

@ChristopherDavenport ChristopherDavenport Jul 25, 2020

Choose a reason for hiding this comment

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

First connection is not reused. Can have stricter timeouts.

.evalTap {
case Right((request, response)) => send(socket)(Some(request), response)
case Left(err) => send(socket)(None, onError(err))
}
.takeWhile {
Copy link
Member Author

@ChristopherDavenport ChristopherDavenport Jul 25, 2020

Choose a reason for hiding this comment

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

if Error occurs or Connection hasClose is present on either request or response terminate the connection.

Making sure we yield if we are still going to continue.

@ChristopherDavenport
Copy link
Member Author

ChristopherDavenport commented Jul 25, 2020

None of my implementations are well modeling the differences between 1.0 and 1.1 instead acting like everything is operating like 1.1. There may be a better way to manage it in this regard.

rossabaker
rossabaker previously approved these changes Aug 6, 2020
Copy link
Member

@rossabaker rossabaker left a comment

👍 on scalafmt

val reqHasClose = req.headers.get(Connection).exists(_.hasClose)
val respConnection = resp.headers.get(Connection)
val connection: Connection =
if (reqHasClose) Connection(NonEmptyList.of("close".ci))
Copy link
Member

@rossabaker rossabaker Aug 6, 2020

Choose a reason for hiding this comment

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

This, and keep-alive, seem like good candidates for constant headers. Headers like these with low cardinality, or a common value, are an area that we could better optimize than we do today.

@rossabaker rossabaker added this to the 0.21.7 milestone Aug 7, 2020
@rossabaker rossabaker dismissed their stale review Aug 7, 2020

This was a MiMa failure, too

Copy link
Member

@rossabaker rossabaker left a comment

MiMa failure. That's two branches I've misdiagnosed.

Copy link
Member

@rossabaker rossabaker left a comment

Fixed binary compatibility.

@rossabaker rossabaker merged commit 6d09faa into http4s:series/0.21 Aug 8, 2020
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.

None yet

2 participants