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

Optimize some memory allocations from ActorPath. #4351

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

petrikero
Copy link
Contributor

I was seeing a lot of allocations happening in a system with tens of thousands of actors, which was causing the system to slow down over hours or days due to the increase of time spent in GC. This PR optimizes away some allocations from commonly used ActorPath methods, in order of importance.

  1. Added ChildActorPath.GetHashCode(), which is able to compute the hash code without invoking ActorPath.Elements, which allocates memory for the segments. The implementation iterates the segments in reverse order compared to the original implementation. It didn't seem to cause any problems, but I might not understand all the repercussions. It is possible to go back to the original order, but it requires some additional trickery.

The GetHashCode() code path is invoked a lot in the code due to the various caches using Dictionary<IActorRef, xxx>. Many of the Dictionary methods invoke GetHashCode() on the key.

I needed to add the newly added GetHashCode() to CoreAPISpec.ApproveCore.approved.txt, though it's not really a public API change.

  1. Optimized ActorPath.Equals(ActorPath other) to avoid getting Elements property (which allocates). The new implementation should behave exactly like the old one.

This mostly triggers when comparing identical ActorPaths which are different objects, as the Uid comparison early-exits non-matching comparisons.

  1. Changed ActorPath.Validate() to take a string instead of char[]. This avoids an allocation due to not needing to call string.ToCharArray(). Some micro-benchmarks show ToCharArray() a tiny bit faster, but less GC pressure is arguably more desirable behavior. There might be faster still variants if fixed(char* p) -style tricks are used.

  2. The ActorPath.SystemElements and UserElements weren't used anywhere, so I removed them.

@Aaronontheweb Aaronontheweb self-requested a review March 23, 2020 13:15
@Aaronontheweb Aaronontheweb added this to the 1.4.4 milestone Mar 23, 2020
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb Aaronontheweb merged commit 5034cc3 into akkadotnet:dev Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants