-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Optimize allocations in ActorPath.Join() #4510
Optimize allocations in ActorPath.Join() #4510
Conversation
…ments causes quite a bit of garbage).
I tried looking at some of the failing logs, but as far as I can tell, all the errors seem unrelated. There's a lot of logs, though, so I may have easily missed something real. |
Before: BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.388 (2004/?/20H1)
|
After: // * Summary * BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.388 (2004/?/20H1)
|
I believe |
Yep akka.net/src/core/Akka/Actor/ActorPath.cs Lines 537 to 543 in 602013c
Looks like this PR improves the speed of that method by up ~22% - nicely done. |
@petrikero yeah, those errors were unrelated - we have a large test suite and some of the tests are timing-sensitive (assert that work gets done within a bounded window of time) and those are notoriously brittle. We've been going through and hardening some, disabling others. An ongoing battle for the maintenance team here. Thanks for this PR though - this is terrific. |
Optimize ActorPath.Join() to perform fewer allocations. Accessing Elements causes quite a bit of garbage to get generated.
In .NET Core or .NET Standard 2.1, the char[] buffer allocation could be avoided, but I'm sure this isn't good enough reason to require either.
The calls are mainly triggered by serialization of ActorRefs when running with Akka.Remote. We have thousands of actors running on each node and sometimes sending ActorRefs over the wire. We're mostly sending unique ActorRefs (each gets sent only once), so the caching doesn't help.