You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "He-Pin (via GitHub)" <gi...@apache.org> on 2025/12/07 10:28:03 UTC
[PR] chore: optimize Source#future and Source#futureSource [pekko]
He-Pin opened a new pull request, #2560:
URL: https://github.com/apache/pekko/pull/2560
Motivation:
The TraversalBuilder has some optimization around the `Single` and `Empty` source. so convert them it single/empty if possible will help performance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #2560:
URL: https://github.com/apache/pekko/pull/2560#discussion_r2616359423
##########
stream/src/main/scala/org/apache/pekko/stream/scaladsl/Source.scala:
##########
@@ -567,8 +561,12 @@ object Source {
* Emits a single value when the given `Future` is successfully completed and then completes the stream.
* The stream fails if the `Future` is completed with a failure.
*/
- def future[T](futureElement: Future[T]): Source[T, NotUsed] =
- fromGraph(new FutureSource[T](futureElement))
+ def future[T](futureElement: Future[T]): Source[T, NotUsed] = futureElement.value match {
+ case Some(scala.util.Success(null)) => empty[T]
Review Comment:
Would be better to check once for if the future has already completed? If it has then check the various success cases. Also, how about checking if it's already failed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2560:
URL: https://github.com/apache/pekko/pull/2560#issuecomment-3649053127
@mdedetrich @pjfanning @raboof Would you like to give a review on this, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #2560:
URL: https://github.com/apache/pekko/pull/2560#discussion_r2616378229
##########
stream/src/main/scala/org/apache/pekko/stream/scaladsl/Source.scala:
##########
@@ -567,8 +561,12 @@ object Source {
* Emits a single value when the given `Future` is successfully completed and then completes the stream.
* The stream fails if the `Future` is completed with a failure.
*/
- def future[T](futureElement: Future[T]): Source[T, NotUsed] =
- fromGraph(new FutureSource[T](futureElement))
+ def future[T](futureElement: Future[T]): Source[T, NotUsed] = futureElement.value match {
+ case Some(scala.util.Success(null)) => empty[T]
Review Comment:
Wow, Nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #2560:
URL: https://github.com/apache/pekko/pull/2560
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2560:
URL: https://github.com/apache/pekko/pull/2560#issuecomment-3649569374
@pjfanning I have updated with your suggestion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #2560:
URL: https://github.com/apache/pekko/pull/2560#discussion_r2616395932
##########
stream/src/main/scala/org/apache/pekko/stream/scaladsl/Source.scala:
##########
@@ -589,8 +589,17 @@ object Source {
* Emits a single value when the given `Future` is successfully completed and then completes the stream.
* The stream fails if the `Future` is completed with a failure.
*/
- def future[T](futureElement: Future[T]): Source[T, NotUsed] =
- fromGraph(new FutureSource[T](futureElement))
+ def future[T](futureElement: Future[T]): Source[T, NotUsed] = {
+ val maybeValue = futureElement.value
+ if (maybeValue eq None) {
+ fromGraph(new FutureSource[T](futureElement))
+ } else futureElement.value match {
Review Comment:
Use maybeValue param
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #2560:
URL: https://github.com/apache/pekko/pull/2560#discussion_r2616366994
##########
stream/src/main/scala/org/apache/pekko/stream/scaladsl/Source.scala:
##########
@@ -567,8 +561,12 @@ object Source {
* Emits a single value when the given `Future` is successfully completed and then completes the stream.
* The stream fails if the `Future` is completed with a failure.
*/
- def future[T](futureElement: Future[T]): Source[T, NotUsed] =
- fromGraph(new FutureSource[T](futureElement))
+ def future[T](futureElement: Future[T]): Source[T, NotUsed] = futureElement.value match {
+ case Some(scala.util.Success(null)) => empty[T]
Review Comment:
The Scala compiler might produce an optimized if or switch but I'd prefer if the code here explicitly checked for future element.value == None first as this is the most likely result. Checking this last could lead to unnecessary extra checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #2560:
URL: https://github.com/apache/pekko/pull/2560#issuecomment-3649167843
I'm still suspicious about this and other recent performance oriented changes. They introduce more checks which reduces performance. These checks include instanceof checks which are not trivial. The performance gains are not proven. No microbenchmarks, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #2560:
URL: https://github.com/apache/pekko/pull/2560#issuecomment-3649841319
> I'm still suspicious about this and other recent performance oriented changes. They introduce more checks which reduces performance. These checks include instanceof checks which are not trivial. The performance gains are not proven. No microbenchmarks, etc.
I haven't looked into the details of this PR, but from what I can tell its just doing specialization which in general is fine premise especially with data structures that time time to initialize/have an expensive initial cost. Having 0/1/2 sized streams is very common (2 sized streams is common due to tuples which `SubFlow`/`SubSource` use internally) so this can provide real benefits
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #2560:
URL: https://github.com/apache/pekko/pull/2560#discussion_r2616552763
##########
stream/src/main/scala/org/apache/pekko/stream/scaladsl/Source.scala:
##########
@@ -589,8 +589,17 @@ object Source {
* Emits a single value when the given `Future` is successfully completed and then completes the stream.
* The stream fails if the `Future` is completed with a failure.
*/
- def future[T](futureElement: Future[T]): Source[T, NotUsed] =
- fromGraph(new FutureSource[T](futureElement))
+ def future[T](futureElement: Future[T]): Source[T, NotUsed] = {
+ val maybeValue = futureElement.value
+ if (maybeValue eq None) {
+ fromGraph(new FutureSource[T](futureElement))
+ } else futureElement.value match {
Review Comment:
I just found I should write all in pattern matching, instead of this wired if else.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #2560:
URL: https://github.com/apache/pekko/pull/2560#discussion_r2616483299
##########
stream/src/main/scala/org/apache/pekko/stream/scaladsl/Source.scala:
##########
@@ -589,8 +589,17 @@ object Source {
* Emits a single value when the given `Future` is successfully completed and then completes the stream.
* The stream fails if the `Future` is completed with a failure.
*/
- def future[T](futureElement: Future[T]): Source[T, NotUsed] =
- fromGraph(new FutureSource[T](futureElement))
+ def future[T](futureElement: Future[T]): Source[T, NotUsed] = {
+ val maybeValue = futureElement.value
+ if (maybeValue eq None) {
+ fromGraph(new FutureSource[T](futureElement))
+ } else futureElement.value match {
Review Comment:
Opps，would you push directly on this,I am away from computure, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2560:
URL: https://github.com/apache/pekko/pull/2560#issuecomment-3649850280
I just found I should have written it all in pattern matching, instead of this wired if else one....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] chore: optimize Source#future and Source#futureSource [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #2560:
URL: https://github.com/apache/pekko/pull/2560#issuecomment-3649197079
This primarily compares the process to materializing an Actor. Although some checks are added here, it essentially involves only a few bytecode instructions, making it significantly lighter than actually materializing an Actor. At the same time, simplifying a Stage to Empty or Single provides more optimization opportunities within the TraversalBuilder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org