-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[Ready] Modified Akka.Remote.Transport benchmarks to get more accurate readings #1560
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
Conversation
16b0378
to
318b5c6
Compare
{ | ||
public static Config ThreadPoolDispatcherConfig => ConfigurationFactory.ParseString(@" | ||
akka.remote.default-remote-dispatcher { | ||
type = Dispatcher |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might need someone else to verify this, but this is the correct way to specify the ThreadPoolDispatcher
in HOCON, no?
Putting the WIP tag on this, as it was able to immediately reproduce the deadlock that I thought I fixed on the And the errors with the |
Going to continue tinkering with this, but I could use some help from @akkadotnet/contributors on resolving #1547 as part of this PR. It could be any one of the following:
The bug occurs here inside the spec: [PerfSetup]
public void Setup(BenchmarkContext context)
{
_remoteMessageThroughput = context.GetCounter(RemoteMessageCounterName);
System1 = ActorSystem.Create("SystemA" + Counter.Next(), CreateActorSystemConfig("SystemA" + Counter.Current, "127.0.0.1", 0));
_echo = System1.ActorOf(Props.Create(() => new EchoActor()), "echo");
System2 = ActorSystem.Create("SystemB" + Counter.Next(), CreateActorSystemConfig("SystemB" + Counter.Current, "127.0.0.1", 0));
_receiver =
System2.ActorOf(
Props.Create(() => new BenchmarkActor(_remoteMessageThroughput, RemoteMessageCount, _resetEvent)),
"benchmark");
var system1Address = RARP.For(System1).Provider.Transport.DefaultAddress;
var system2Address = RARP.For(System2).Provider.Transport.DefaultAddress;
var system1EchoActorPath = new RootActorPath(system1Address) / "user" / "echo";
var system2RemoteActorPath = new RootActorPath(system2Address) / "user" / "benchmark";
_remoteReceiver =
System1.ActorSelection(system2RemoteActorPath).Ask<ActorIdentity>(new Identify(null), TimeSpan.FromSeconds(2)).Result.Subject;
_remoteEcho =
System2.ActorSelection(system1EchoActorPath)
.Ask<ActorIdentity>(new Identify(null), TimeSpan.FromSeconds(2))
.Result.Subject;
} The |
I think I've figured out #1547 - going to run the perf tests multiple times on the build server so we can be confident that it's resolved now. |
3f51b1b
to
f5df519
Compare
Ready to be merged. |
|
||
System2.ActorSelection(system1EchoActorPath).Ask<string>("foo").ContinueWith(tr => | ||
{ | ||
Console.WriteLine("Received foo"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats the purpose of this line? are there some stdout capture going on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good catch - added it when I was debugging. I'll remove it.
var remoteTask1 = System1.ActorSelection(system2RemoteActorPath).Ask<ActorIdentity>(new Identify(null)); | ||
remoteTask1.Wait(); | ||
_remoteReceiver = remoteTask1.Result.Subject; | ||
_remoteEcho = | ||
System2.ActorSelection(system1EchoActorPath) | ||
.Ask<ActorIdentity>(new Identify(null), TimeSpan.FromSeconds(2)) | ||
.Result.Subject; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System2.ActorSelection(system1EchoActorPath).ResolveOne().Result.Subject
would be more idiomatic.
Might not matter in the test but I've seen the above pop up a few times before.
Modified Akka.Actor benchmarks to get more accurate reading Added dispatcher-specific specs for Akka.Remote close akkadotnet#1547 - fixed benchmarks that depend on `TestTransport` close akkadotnet#1556 - memory leak caused by not clearing `TestTransport` logs fixed shutdown spam errors added association stress test
f5df519
to
95fed6c
Compare
[Ready] Modified Akka.Remote.Transport benchmarks to get more accurate readings
Modified Akka.Actor benchmarks to get more accurate reading
Added dispatcher-specific specs for Akka.Remote
close #1547 - fixed benchmarks that depend on
TestTransport
close #1556 - memory leak caused by not clearing
TestTransport
logs