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

Improve performances: Remove the aggregateAsync call #749

Closed
wants to merge 1 commit into from

Conversation

guizmaii
Copy link
Member

@guizmaii guizmaii commented Mar 26, 2023

To replicate the benchmark results:

sbt "zioKafkaBench/Jmh/run -wi 10 -i 10 -r 1 -w 1 -t 1 -f 5 -foe true zio.kafka.bench.ConsumersComparisonBenchmark"

Before (master branch):

[info] Result "zio.kafka.bench.ConsumersComparisonBenchmark.zioKafka":
[info]   2347.283 ±(99.9%) 495.618 ms/op [Average]
[info]   (min, avg, max) = (1143.906, 2347.283, 4427.192), stdev = 1001.173
[info]   CI (99.9%): [1851.665, 2842.901] (assumes normal distribution)
[info] # Run complete. Total time: 00:07:59

After:

[info] Result "zio.kafka.bench.ConsumersComparisonBenchmark.zioKafka":
[info]   1796.335 ±(99.9%) 161.723 ms/op [Average]
[info]   (min, avg, max) = (1169.957, 1796.335, 2808.878), stdev = 326.688
[info]   CI (99.9%): [1634.612, 1958.057] (assumes normal distribution)
[info] # Run complete. Total time: 00:06:36

IMO, these results are (I could be wrong, ofc) quite counter-intuitive/surprising.
This aggregateAsync call seems to be an optimisation: It reduces the number of operations for each message, some of these operations being done once per batch instead of once per message.
Therefore, IMO, it's quite surprising that removing this call has such a positive impact on the performances. 🤔

@adamgfraser Would you expect such results? Is there a runFoldChunkZio that could allow us to keep the chunking?

Cc @svroonland @erikvanoosten

Edit:
FYI, I made an experiment here to keep the chunking: #751
The results are surprising too. The peak performance is better but the variability in the results is quite high

@guizmaii guizmaii requested a review from iravid as a code owner March 26, 2023 19:33
@guizmaii
Copy link
Member Author

Closing in favour of #751

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

1 participant