-
Notifications
You must be signed in to change notification settings - Fork 595
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
Stack of potential HTTP/2 improvements #3822
Conversation
Test FAILed. Pull request validation reportFailed Test SuitesTest result for
|
def tearDown(): Unit = { | ||
system.terminate() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you mean to delete this one, or did it move?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks probably broken merge.
data.runWith(subIn.sink)(subFusingMaterializer) | ||
case Left(data) => | ||
info.addAllData(data) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
@@ -44,7 +44,7 @@ private[http2] object HeaderCompression extends GraphStage[FlowShape[FrameEvent, | |||
case ParsedHeadersFrame(streamId, endStream, kvs, prioInfo) => | |||
kvs.foreach { | |||
case (key, value) => | |||
encoder.encodeHeader(os, key.getBytes(HeaderDecompression.UTF8), value.getBytes(HeaderDecompression.UTF8), false) | |||
encoder.encodeHeader(os, key.getBytes(HeaderDecompression.US_ASCII), value.getBytes(HeaderDecompression.US_ASCII), false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for ascii it's possible to take it one step further by using this in Akka:
https://github.com/akka/akka/blob/206dafa01d81224e5f93ddc6e6ec7fa702a03ce3/akka-actor/src/main/scala/akka/util/Unsafe.java#L129
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, interesting, I think we have a similar helper already also in akka-http as well.
f3ab2df
to
595dc45
Compare
Test FAILed. Pull request validation reportMima Failures
|
Added the reverse optimization as well by adding a new option |
595dc45
to
95b11b7
Compare
Test FAILed. |
95b11b7
to
f6aeeb6
Compare
Test PASSed. |
(cherry picked from commit ecf412a2eb11de0cb9b06188555b82271d3fdc37) # Conflicts: # akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpMessageParser.scala # akka-http-core/src/test/scala/akka/http/scaladsl/TestServer.scala (cherry picked from commit e9311f64ea788d69ff4def566f36dac1b33bd670)
43b15cc
to
090b152
Compare
Test FAILed. !!! Couldn't read commit file !!! |
Test FAILed. !!! Couldn't read commit file !!! |
Most of the interesting parts from here have been merged in the meantime. |
So far this is somewhat of a grab bag. Will create separate PRs later.
Planned further optimizations: