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

feat: improve perf of groupMessagesForPartition (~600x in production use-case) #1576

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benvan
Copy link
Contributor

@benvan benvan commented May 13, 2023

This PR is the result of diagnosing high cpu usage on our ECS cluster.

It rewrites the src/producer/groupMessagesPerPartition.js implementation to be O(n)
In practice, this results in a significant performance improvement.

In our use-case, we regularly have blocks of many thousands of messages (20k was the scenario that led to this diagnostic).

I'll lead with the goods:

The following runs execute groupMessagesPerPartition with the specified number of messages.

The tests were run on an ECS Fargate instance with 2vCPU allocated.

Running for 10 messages:
old: 0.93ms
new: 0.49ms
improvement: 1.9x

Running for 50 messages:
old: 0.75ms
new: 0.20ms
improvement: 3.7x

Running for 100 messages:
old: 0.63ms
new: 0.21ms
improvement: 2.9x

Running for 500 messages:
old: 1.13ms
new: 0.085ms
improvement: 13.3x

Running for 1000 messages:
old: 4.73ms
new: 0.097ms
improvement: 48.6x

Running for 10000 messages:
old: 135.59ms
new: 3.33ms
improvement: 40.6x

Running for 20000 messages:
old: 1946.62ms
new: 3.23ms
improvement: 602.0x

Running for 50000 messages:
old: 16917.32ms
new: 0.97ms
improvement: 17415.6x

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.

4 participants