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

Request reply baseline benchmark #453

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Request reply baseline benchmark #453

wants to merge 9 commits into from

Conversation

mtmk
Copy link
Collaborator

@mtmk mtmk commented Mar 24, 2024

Request reply performance improvements: The basic idea is to create a shortcut to avoid additional subscriber (albeit muxed) creation so that we can save on additional allocations and potentially work. reply many can stay the same since creating a subscriber makes sense there.

This is a baseline benchmark we can merge now so that when the actual performance work is done we can have an easy way to compare results:

Method Mean Error StdDev Allocated
RequestReplyAsync 217.4 us 41.71 us 2.29 us 3.68 KB

@mtmk mtmk marked this pull request as draft March 28, 2024 13:26
@stebet
Copy link
Contributor

stebet commented Apr 3, 2024

@mtmk One thing I did in my app that uses the v2 lib is to not use the built-in request-reply since it creates a new subscription for every request. I just use one subscription generating a random prefix (inbox if you will) and then reuse that subscription for all requests, appending a unique ID to the prefix, f.ex:
_somerandominboxid.randomrequestid.

That way my custom subscriber just has to have one active long-running ephemeral subscription to get each response for _somerandominboxid.*.

It performs a lot better than the built-in request-reply mechanism.

Is there a reason it currently creates a new subscription each time?

@mtmk
Copy link
Collaborator Author

mtmk commented Apr 3, 2024

@stebet what you're doing is sensible and we should handle it more efficiently in the library. It's not as bad at the moment since we are using a 'mux' inbox as you described, but still creating somewhat a dummy 'inbox subscription' object to handle it. So the communication with the server is optimal but internal handling still a bit costly.

The reason for this approach is to generalize mux subscriptions so application code can leverage that too. e.g. if you create a subscription with the current inbox prefix, that would not create a new subscription on the server. However, this is not documented and I'm not sure if it's used at all.

So the idea in this PR to make a special case for Request-Reply since it's a very common pattern (especially when using JetStream for example) and improve its 'internal' performance. Any ideas, critique welcome, as always.

@mtmk mtmk self-assigned this Apr 25, 2024
@mtmk mtmk changed the title Request reply benchmarks Request reply baseline benchmark May 5, 2024
@mtmk mtmk requested a review from caleblloyd May 5, 2024 17:18
@mtmk
Copy link
Collaborator Author

mtmk commented May 5, 2024

keeping this PR simple to a benchmark only.

@mtmk mtmk marked this pull request as ready for review May 5, 2024 17:19
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

2 participants