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

Fix performance of HTTP server (for strict entities at least) #19255

Merged
merged 2 commits into from Jan 8, 2016

Conversation

jrudolph
Copy link
Member

i.e. only for Strict entities where we can now completely get rid of per-request materialization.

This includes a fix for #19240 and also the corresponding issue on the rendering side.

This is just a quick try which makes a few tests fail probably because some boundary conditions need to be fixed properly.

With this patch performance is up to ~ 90k RPS (from 15k without the patch) on my machine.

/cc @sirthias

@jrudolph jrudolph force-pushed the w/19240-http-remove-flattenMerges branch from 630bc35 to eee55c6 Compare December 21, 2015 19:06
@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR is currently being validated by Jenkins labels Dec 21, 2015
@akka-ci
Copy link

akka-ci commented Dec 21, 2015

Test FAILed.

@akka-ci akka-ci added the needs-attention Indicates a PR validation failure (set by CI infrastructure) label Dec 21, 2015
@akka-ci
Copy link

akka-ci commented Dec 21, 2015

Test FAILed.

@jrudolph jrudolph force-pushed the w/19240-http-remove-flattenMerges branch from eee55c6 to fa6b36a Compare January 5, 2016 11:28
@jrudolph jrudolph changed the title PREVIEW fix performance of HTTP server Fix performance of HTTP server (for strict entities at least) Jan 5, 2016
@jrudolph
Copy link
Member Author

jrudolph commented Jan 5, 2016

Cleaned up and rebased (also I added an issue for the second part: #19352).

It is interesting that you don't need to actually touch the stage functionality (on both sides). The only thing that is needed is some slight additions to the already existing GraphStages that handle the substreams properly. I wonder if some form of this would even be generic enough to be included in GraphStages themselves as helper methods.

On the rendering side the only thing needed would be an emit(Source) which would transfer all elements from the Source to the given Outlet.

On the parsing side, something similar is needed, i.e. which creates a substream, wraps that substream (in an HttpEntity in that case) and pushes it and then transfers input elements to that substream as long as (or until) some predicate holds.

@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels Jan 5, 2016
@akka-ci
Copy link

akka-ci commented Jan 5, 2016

Test FAILed.

@jrudolph
Copy link
Member Author

jrudolph commented Jan 5, 2016

Here is the current benchmark with and without this PR:

RPS Fusing enabled What
23k no akka-http 2.0.1
23k yes akka-http 2.0.1
67k no This PR
115k yes This PR

It turns out that fusing has no effect in the original version because basically all the time is spent in materialization. Only after per-request materialization was removed the benefits of fusing can actually reap their benefits.

@jrudolph
Copy link
Member Author

jrudolph commented Jan 5, 2016

The test failure was in akka.stream.scaladsl.QueueSinkSpec, so most likely unrelated.

@ktoso
Copy link
Member

ktoso commented Jan 5, 2016

Those look very promising, and also seem to overlap with what Roland observed before xmas when benchmarking (that things did not make much of a difference then). Thanks a lot!
I'll want to read the first commit here in detail before merging, will do that soon :)

@ktoso
Copy link
Member

ktoso commented Jan 5, 2016

PLS BUILD

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels Jan 5, 2016
@akka-ci
Copy link

akka-ci commented Jan 5, 2016

Test PASSed.

@ktoso
Copy link
Member

ktoso commented Jan 8, 2016

Promising to (1st commit, 2nd already done) review (and hope to merge) today.

@ktoso ktoso added the 2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding label Jan 8, 2016
@ktoso
Copy link
Member

ktoso commented Jan 8, 2016

Finally read through the entire thing, looks good! Thanks a lot and sorry for the delay @jrudolph!

ktoso added a commit that referenced this pull request Jan 8, 2016
Fix performance of HTTP server (for strict entities at least)
@ktoso ktoso merged commit 9f964ef into akka:release-2.3-dev Jan 8, 2016
@jrudolph
Copy link
Member Author

Cool, thanks for reviewing and merging, @ktoso.

@jrudolph jrudolph deleted the w/19240-http-remove-flattenMerges branch January 10, 2016 10:35
ktoso added a commit to ktoso/akka that referenced this pull request Jan 11, 2016
…erges

Fix performance of HTTP server (for strict entities at least)
@ktoso ktoso removed the 2 - pick next Used to mark issues which are next up in the queue to be worked on. The tag is non-binding label Jan 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants