[V2] ReceiveActor - Removing MatchBuilder and making use of handlers. #7556
Conversation
This is probably not the most ideal code. I want to make sure the tests are passing better before getting to cleaning it up
Still quite a bit of understanding of how the MatchBuilder was working
…ed `Receive<T>` ordering a continuation of akkadotnet#7498
| [Theory] | ||
| [InlineData(true)] | ||
| [InlineData(false)] | ||
| public void Given_TypedReceiveHandler_can_match_interface_on_ConcreteTypes(bool usePredicate) |
There was a problem hiding this comment.
This spec at the bottom covers two scenarios:
- Tests the preservation of Akka.NET (,1.5]
Receive<T>declaration order - Asserts that your
Type-assignability code from ReceiveActor - Removing MatchBuilder and making use of handlers. #7498 works
| HandleAny = null; | ||
| } | ||
|
|
||
| private List<ITypeHandler> TypedHandlers { get; } |
There was a problem hiding this comment.
Moved from a Dictionary to a List in order to guarantee that declared Receive<T> order matches our handling order, which is an important behavioral requirement
| TypedHandlers.Add(CreateTypeHandler(messageType, shouldHandlePredicate, handler)); | ||
|
|
||
| // If the message type is object, then we need to track that we have added a handler with no predicate. | ||
| if (messageType == typeof(object) && |
There was a problem hiding this comment.
In theory we could also do this from AddGenericReceiveHandler<T> but IMHO we're better off authoring an analyzer and flagging this issue at compilation - we have the ability to do that easily now through Akka.Analyzers.
| HandleAny = handler; | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] |
There was a problem hiding this comment.
A little prayer to the compiler to make this faster
| bool TryHandle(object message); | ||
| } | ||
|
|
||
| internal sealed class WeaklyTypedPredicateHandler : ITypeHandler |
There was a problem hiding this comment.
Better to have 4 purpose-built ITypeHandlers so we can keep the TryHandle as slim as possible. If it turns out that a predicate null check has lower perf impact that the interface dispatcher overhead then we can pare this down to 2 implementations. If we get rid of the non-generic Receive methods from receive actors then we can get this down to a single implementation and remove the interface altogether.
lol
|
|
Getting some BDN numbers too since we have a |
dev
This PR
|
|
I need to adjust the benchmark config with BDN to look a bit closer to what we did for Akka.Streams.Kafka, but overall it doesn't look like these changes hurt our perf at all. |
|
So I ran this PR against #7458 and it didn't reduce the AOT warnings at all, but for a very obvious reason: the config and |
|
Superseded via #7557 |
2 participants
Changes
A continuation of @mhbuck 's work on #7498 - addressing some of the comments I made in my review of that PR.
Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
Latest
devBenchmarksThis PR's Benchmarks