-
Notifications
You must be signed in to change notification settings - Fork 156
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
feat: Make SingleConsumerMultiProducer the default mailbox for stream. #917
Conversation
stream/src/main/scala/org/apache/pekko/stream/impl/ActorMaterializerImpl.scala
Outdated
Show resolved
Hide resolved
stream/src/main/scala/org/apache/pekko/stream/impl/PhasedFusingActorMaterializer.scala
Outdated
Show resolved
Hide resolved
stream/src/main/scala/org/apache/pekko/stream/impl/ActorMaterializerImpl.scala
Show resolved
Hide resolved
stream/src/main/scala/org/apache/pekko/stream/impl/PhasedFusingActorMaterializer.scala
Show resolved
Hide resolved
It seems that a default configuration has been added. Are there any unit test? |
@laglangyue I don't this this code need any test, and a test may need another pr first:), you can try with a debuger to see the mailboxtype. |
lgtm |
@mdedetrich Would you like to give this a review? this was landed in Akka 2.7 |
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.
lgtm. It may be worth it to create a simple unit test that confirms PhasedFusingActorMaterializer.MailboxConfigName
is actually being used when a stream is run, but if this is too hard then don't worry about it.
@mdedetrich Not easy to test, that's why I want the #919 . and I did confirm that works with debuger. |
No worries, I approved the PR anyways so its good from my end |
Motivation:
Make stream actor using the SingleConsumerMultiProducer by default for performance.
refs: #915