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

perf: Reducing memory cost while replaying #765

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

Roiocam
Copy link
Contributor

@Roiocam Roiocam commented Sep 14, 2023

No References.

Motivation:

Reducing memory cost on replaying query stream

Result

Additional bonus: slight performance improvement.

jmh:run -i 3 -wi 3 -f 1 .JdbcAsyncJournalBenchmark

[info] Benchmark                                     Mode  Cnt       Score       Error  Units
[info] JdbcAsyncJournalBenchmark.originalAsync      thrpt    3  590394.058 ± 38433.920  ops/s
[info] JdbcAsyncJournalBenchmark.streamWithCollect  thrpt    3  594978.499 ±  2034.776  ops/s

[info] Benchmark                                     Mode  Cnt       Score       Error  Units
[info] JdbcAsyncJournalBenchmark.originalAsync      thrpt    3  584438.653 ± 35769.421  ops/s
[info] JdbcAsyncJournalBenchmark.streamWithCollect  thrpt    3  587042.327 ± 15553.554  ops/s

JMH for this change: https://gist.github.com/Roiocam/aabbf404aa2e1a4e967967e450b0def0

@@ -118,7 +118,7 @@ class JdbcAsyncWriteJournal(config: Config) extends AsyncWriteJournal {
journalDao
.messagesWithBatch(persistenceId, fromSequenceNr, toSequenceNr, journalConfig.daoConfig.replayBatchSize, None)
.take(max)
.mapAsync(1)(reprAndOrdNr => Future.fromTry(reprAndOrdNr))
.collect { case Success(reprAndOrdNr) => reprAndOrdNr }
Copy link
Member

Choose a reason for hiding this comment

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

This has different behaviour than the original logic though, that would fail the stream on a Failure while this filters it out. Better would be to skip this operator/step completely and do the match in runForEach and throw for Failure there:

      .runForeach { 
        case Success((repr, _)) =>
          recoveryCallback(repr)
        case Failure(ex) => throw ex
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More effiency, Yay!

[info] Benchmark                                     Mode  Cnt       Score        Error  Units
[info] JdbcAsyncJournalBenchmark.originalAsync      thrpt    3  581405.864 ± 217516.676  ops/s
[info] JdbcAsyncJournalBenchmark.streamWithCollect  thrpt    3  594420.619 ±  29680.962  ops/s

[info] Benchmark                                     Mode  Cnt       Score       Error  Units
[info] JdbcAsyncJournalBenchmark.originalAsync      thrpt    3  577291.504 ± 51386.930  ops/s
[info] JdbcAsyncJournalBenchmark.streamWithCollect  thrpt    3  605493.214 ±  4152.262  ops/s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Push my commit again, because i havn't permission to apply this request change,

@Roiocam Roiocam force-pushed the roiocam-less-mem-cost branch from 272e013 to 9cbe433 Compare September 14, 2023 12:09
Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@johanandren johanandren merged commit ed541ff into akka:master Sep 14, 2023
@Roiocam Roiocam deleted the roiocam-less-mem-cost branch September 14, 2023 15:30
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.

2 participants