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

Benchmark to test SingleRequestResponseToRemoteEntity with a local proxy #5232

Merged
merged 3 commits into from
Aug 30, 2021

Conversation

carlcamilleri
Copy link
Contributor

Adding benchmark to test performance of sending messages to a remote entity without relying on remote ASK as per the approach discussed at #5203 (comment)

@@ -43,6 +45,50 @@ public class ShardMessageRoutingBenchmarks
private IActorRef _batchActor;
private Task _batchComplete;

#if (DEBUG)
Copy link
Member

Choose a reason for hiding this comment

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

What's this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was using this to debug the functionality of the benchmark (be able to attach breakpoints etc.) as per https://benchmarkdotnet.org/articles/guides/troubleshooting.html - if another approach is usually taken, happy to remove or change as needed

Thanks

Copy link
Member

Choose a reason for hiding this comment

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

Anything different about code in this block vs. the #else block other than async Task vs. void? Was trying to spot some but none jumped out at me. The other statement, with the debugging config added, makes sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct I was running into an issue with async code in debug mode - the benchmark was executing before the task completion. I can refactor for a cleaner approach if it’s better (still would need two methods I believe due to async signature)

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Thank you!

Nope, that's ok - just adding a comment to explain why it's there would be sufficient. We can do that in a subsequent PR.

Thank you for your contribution!

@Aaronontheweb Aaronontheweb merged commit 243d257 into akkadotnet:dev Aug 30, 2021
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.

3 participants