eliminate mailbox delegate allocations #6134
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Aaronontheweb
commented
Oct 5, 2022
| @@ -142,7 +143,7 @@ internal LatestFirstSystemMessageList SystemQueue | |||
| { | |||
| // Note: contrary how it looks, there is no allocation here, as SystemMessageList is a value class and as such | |||
| // it just exists as a typed view during compile-time. The actual return type is still SystemMessage. | |||
| return new LatestFirstSystemMessageList(Volatile.Read(ref _systemQueueDoNotCallMeDirectly)); | |||
There was a problem hiding this comment.
Not needed - field is already marked as volatile
| @@ -201,7 +202,7 @@ public virtual void SetActor(ActorCell actorCell) | |||
| /// TBD | |||
| /// </summary> | |||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | |||
| internal int CurrentStatus() { return Volatile.Read(ref _statusDotNotCallMeDirectly); } | |||
There was a problem hiding this comment.
Not needed - field is already marked as volatile
| @@ -251,7 +252,7 @@ private bool UpdateStatus(int oldStatus, int newStatus) | |||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | |||
| private void SetStatus(int newStatus) | |||
| { | |||
| Volatile.Write(ref _statusDotNotCallMeDirectly, newStatus); | |||
There was a problem hiding this comment.
Not needed - field is already marked as volatile
| if (!IsClosed()) // Volatile read, needed here | ||
| if (IsClosed()) return; // Volatile read, needed here | ||
|
|
||
| var tmp = InternalCurrentActorCellKeeper.Current; |
There was a problem hiding this comment.
Inlined the contents of ActorCell.UseThreadContext here.
Aaronontheweb
added a commit
to Aaronontheweb/akka.net
that referenced
this pull request
Oct 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Reduces delegate allocations while processing actor mailbox contents to zero - eliminates the vast majority of allocations that occur during mailbox processing.
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
Latest
v1.4BenchmarksRunning on .NET 6 - note: I modified the
MailboxThroughputBenchmarksto only use theCallingThreadDispatcheras that's a cleaner signal for measuring throughput impact of these changes, which otherwise won't show up as easily using the .NET ThreadPool.This PR's Benchmarks
Perf Summary
Reduces allocations on 100,000 msgs processed from 203kb to 1,008 bytes. Improves throughput slightly.