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

SSE Performance with big events #1508

Closed
kennedyoliveira opened this issue Nov 1, 2017 · 14 comments
Closed

SSE Performance with big events #1508

kennedyoliveira opened this issue Nov 1, 2017 · 14 comments
Assignees
Labels
3 - in progress Someone is working on this ticket help wanted Identifies issues that the core team will likely not have time to work on t:stream
Milestone

Comments

@kennedyoliveira
Copy link
Contributor

Hi!

While consuming events from the Marathon SSE via Alpakka SSE Connector, i noticed it wasn't consuming some events and instead closing the connection, investigating a bit, i found that the akka.http.scaladsl.unmarshalling.sse.LineParser used to feed lines in the EventStreamParser takes to long to parse a line who is big, i know usually the sse doesn't send big lines, but Marathon send some events that have 1 ~ 2 mb, and it takes too long to parse the line.

I mentioned the Marathon because is my use case, but any big event will do.

Is there a alternative for consuming big events?

@ktoso
Copy link
Member

ktoso commented Nov 1, 2017

What specifically is "long" and what is desired; how long do you think is achievable with a raw impl, and how much part of that is that the events are being pulled in slowly?

Please provide more details so this can be acted upon accordingly

@kennedyoliveira
Copy link
Contributor Author

kennedyoliveira commented Nov 1, 2017

By long i mean 1 ~ 2 MB per event, the desired is to parse it fast, currently it can't be parsed in 1 minute and the TCP timeout, i don't know how much time is needed with the current implementation.

What do you mean by how long do you think is achievable with a raw impl?

@viktorklang
Copy link
Member

viktorklang commented Nov 1, 2017 via email

@kennedyoliveira
Copy link
Contributor Author

@viktorklang i don't have, i will write one and post the link here

kennedyoliveira added a commit to kennedyoliveira/akka-http that referenced this issue Nov 2, 2017
kennedyoliveira added a commit to kennedyoliveira/akka-http that referenced this issue Nov 2, 2017
@kennedyoliveira
Copy link
Contributor Author

Hey @viktorklang i created a simple benchmark akka-http-lineparser-bench and was able to improve the performance of the parser via PR #1510. Here are some benchmark numbers i got from running the bench on my machine:

[info] Benchmark                     (chunkSize)  (lineSize)  Mode  Cnt     Score    Error  Units
[info] LineParserBenchmark.baseline          512        1024  avgt    5     0,119 ±  0,007  ms/op
[info] LineParserBenchmark.baseline          512      130945  avgt    5  7360,851 ± 48,572  ms/op
[info] LineParserBenchmark.baseline         1024        1024  avgt    5     0,103 ±  0,007  ms/op
[info] LineParserBenchmark.baseline         1024      130945  avgt    5  1860,797 ± 25,990  ms/op
[info] LineParserBenchmark.baseline         2048        1024  avgt    5     0,086 ±  0,007  ms/op
[info] LineParserBenchmark.baseline         2048      130945  avgt    5   461,925 ±  6,888  ms/op
[info] LineParserBenchmark.baseline         4096        1024  avgt    5     0,085 ±  0,010  ms/op
[info] LineParserBenchmark.baseline         4096      130945  avgt    5    59,315 ±  3,907  ms/op
[info] LineParserBenchmark.baseline         8192        1024  avgt    5     0,084 ±  0,002  ms/op
[info] LineParserBenchmark.baseline         8192      130945  avgt    5    20,337 ±  2,013  ms/op
[info] LineParserBenchmark.improved          512        1024  avgt    5     0,100 ±  0,006  ms/op
[info] LineParserBenchmark.improved          512      130945  avgt    5    90,401 ±  2,068  ms/op
[info] LineParserBenchmark.improved         1024        1024  avgt    5     0,096 ±  0,004  ms/op
[info] LineParserBenchmark.improved         1024      130945  avgt    5    45,914 ±  1,030  ms/op
[info] LineParserBenchmark.improved         2048        1024  avgt    5     0,086 ±  0,009  ms/op
[info] LineParserBenchmark.improved         2048      130945  avgt    5    24,963 ±  4,791  ms/op
[info] LineParserBenchmark.improved         4096        1024  avgt    5     0,086 ±  0,006  ms/op
[info] LineParserBenchmark.improved         4096      130945  avgt    5     6,689 ±  2,497  ms/op
[info] LineParserBenchmark.improved         8192        1024  avgt    5     0,086 ±  0,007  ms/op
[info] LineParserBenchmark.improved         8192      130945  avgt    5     4,046 ±  0,968  ms/op

I did not finished the baseline benchmark for 1 and 2 mb payload because it was taking too long, but for the improved versions here are the numbers:

[info] Benchmark                     (chunkSize)  (lineSize)  Mode  Cnt      Score      Error  Units
[info] LineParserBenchmark.improved          512      523777  avgt    5   1419,675 ±   18,312  ms/op
[info] LineParserBenchmark.improved          512     1129129  avgt    5   8242,736 ±  293,965  ms/op
[info] LineParserBenchmark.improved          512     2095105  avgt    5  31909,511 ± 3866,226  ms/op
[info] LineParserBenchmark.improved         1024      523777  avgt    5    686,950 ±   13,380  ms/op
[info] LineParserBenchmark.improved         1024     1129129  avgt    5   3803,622 ±   72,376  ms/op
[info] LineParserBenchmark.improved         1024     2095105  avgt    5  14205,027 ±  988,739  ms/op
[info] LineParserBenchmark.improved         2048      523777  avgt    5    357,240 ±   10,985  ms/op
[info] LineParserBenchmark.improved         2048     1129129  avgt    5   1832,569 ±  129,886  ms/op
[info] LineParserBenchmark.improved         2048     2095105  avgt    5   5729,783 ±  292,981  ms/op
[info] LineParserBenchmark.improved         4096      523777  avgt    5    180,770 ±    8,955  ms/op
[info] LineParserBenchmark.improved         4096     1129129  avgt    5    817,104 ±   13,516  ms/op
[info] LineParserBenchmark.improved         4096     2095105  avgt    5   3072,116 ±   17,274  ms/op
[info] LineParserBenchmark.improved         8192      523777  avgt    5     83,366 ±    5,222  ms/op
[info] LineParserBenchmark.improved         8192     1129129  avgt    5    428,683 ±  192,236  ms/op
[info] LineParserBenchmark.improved         8192     2095105  avgt    5   1407,533 ±   23,900  ms/op

My machine has the following specs:
CPU => Ryzen 1700x @ 4.0Ghz on all cores 8C/16T
Mem => 32 GB (4x8) DDR4 @ 3200Mhz
SSD => Samsung M2 NVMe 950 Pro
OS => Ubuntu 16.04 x64
JVM => 8u151

Hope it helps!

@ktoso
Copy link
Member

ktoso commented Nov 2, 2017

Lookin good, that's what I was asking for :)
By asking for "long" I meant time, not size of payload.

Could you include that benchmark in the PR as well? It'd just test the existing one ofc, but very good addition to our benchmark suite.

It can be added to akka-bench-jmh

@ktoso ktoso added help wanted Identifies issues that the core team will likely not have time to work on t:stream labels Nov 2, 2017
@kennedyoliveira
Copy link
Contributor Author

@ktoso thanks for the feedback, sure i can add the benchmark, but looking at akka-bench-jmh it doesn't have dependency on akka-http, how should i proceed?

@ktoso
Copy link
Member

ktoso commented Nov 2, 2017

Ah, my bad, missed this was in akka-http; Setting up a bench project inside akka-http would be the best then.

If you're up for it it would be about copying the benchJmh project from https://github.com/akka/akka/blob/master/build.sbt#L88 and adding the plugin https://github.com/akka/akka/blob/master/project/plugins.sbt#L17 then creating akka-bench-jmh and we're all set to have jmh benches here.

@kennedyoliveira
Copy link
Contributor Author

Ah no problem haha, i'll setup the project and update the PR then :)

@ktoso
Copy link
Member

ktoso commented Nov 2, 2017

Awesome, thanks; great contrib

@ktoso ktoso added the 3 - in progress Someone is working on this ticket label Nov 2, 2017
kennedyoliveira added a commit to kennedyoliveira/akka-http that referenced this issue Nov 2, 2017
- Improved performance of LineParser
- Created benchmark subproject
- Added benchmark for LineParser
kennedyoliveira added a commit to kennedyoliveira/akka-http that referenced this issue Nov 2, 2017
- Improved performance of LineParser
- Created benchmark subproject
- Added benchmark for LineParser
kennedyoliveira added a commit to kennedyoliveira/akka-http that referenced this issue Nov 2, 2017
- Improved performance of LineParser
- Created benchmark subproject
- Added benchmark for LineParser
@kennedyoliveira
Copy link
Contributor Author

@ktoso i added the benchmark project with the LineParser benchmark

@kennedyoliveira
Copy link
Contributor Author

Any news?

@ktoso
Copy link
Member

ktoso commented Nov 8, 2017

Please ping in the PR though :-)
As explained in https://github.com/akka/akka/blob/master/CONTRIBUTING.md it needs a 2nd LGTM

jrudolph pushed a commit that referenced this issue Nov 28, 2017
* Improve performance of LineParser for SSE unmarshalling #1508

- Improved performance of LineParser
- Created benchmark subproject
- Added benchmark for LineParser

* Fix parsing of CR ending lines according to the specification

* Reformatting

* Fix line parsing behavior

* Renaming variables
@jrudolph jrudolph added this to the 10.0.11 milestone Nov 28, 2017
@jrudolph
Copy link
Member

Fixed by #1510.

jrudolph added a commit to jrudolph/akka-http that referenced this issue Nov 28, 2017
jrudolph added a commit to jrudolph/akka-http that referenced this issue Nov 28, 2017
jrudolph added a commit that referenced this issue Nov 29, 2017
=htp #1508 cosmetic changes: rename remaining var and move comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - in progress Someone is working on this ticket help wanted Identifies issues that the core team will likely not have time to work on t:stream
Projects
None yet
Development

No branches or pull requests

4 participants