You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "JD557 (via GitHub)" <gi...@apache.org> on 2024/02/22 16:46:22 UTC

[I] Discussion about restoring statefulMapConcat [incubator-pekko]

JD557 opened a new issue, #1141:
URL: https://github.com/apache/incubator-pekko/issues/1141

   I would like to open the discussion to restore `statefulMapConcat`, as a follow up to it's deprecation in https://github.com/apache/incubator-pekko/issues/601 and https://github.com/apache/incubator-pekko-http/issues/395#issuecomment-1916722960
   
   I understand that it had problems regarding the `upstreamFinished`, but the `statefulMap`+`mapConcat` alternative does not seem a viable migration considering the extra allocations.^
   
   I believe this issue was also identified for pekko-http in https://github.com/apache/incubator-pekko-http/issues/395, which lead to https://github.com/apache/incubator-pekko-http/pull/462.
   
   However, I find it odd that the fix is in pekko-http. It seems to me that something like this should be available to all pekko-streams users and be one of the recommended migrations.


-- 
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.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: [I] Discussion about restoring statefulMapConcat [incubator-pekko]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on issue #1141:
URL: https://github.com/apache/incubator-pekko/issues/1141#issuecomment-1959909798

   @JD557 We'll discuss it but be aware that we will not remove the statefulMapConcat method any time soon any way.


-- 
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: [I] Discussion about restoring statefulMapConcat [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #1141:
URL: https://github.com/apache/incubator-pekko/issues/1141#issuecomment-1960821673

   So the quick fix seems just remove the deprecation, and add more detail log in 1.0.x.
   And will need a new building block for this which will not introduce any allocation.


-- 
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: [I] Discussion about restoring statefulMapConcat [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #1141:
URL: https://github.com/apache/incubator-pekko/issues/1141#issuecomment-1959913383

   I know the problem, the only problem of the current implementation is the extra allocation of tuple, which is the same as mapAccumlate in fs2 and zio.
   
   I would like to add some thing as the pr in http, but can't find a good name for now.
   
   For the performance, in test zipwithindex migration, I saw performance boost.
   
   I agree with you with most part but I don't think we should remove the deprecation.
   
   


-- 
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: [I] Discussion about restoring statefulMapConcat [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #1141:
URL: https://github.com/apache/incubator-pekko/issues/1141#issuecomment-1960055388

   I will take care of it this weekend, we will definitely  come up with a solution 😀


-- 
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: [I] Discussion about restoring statefulMapConcat [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on issue #1141:
URL: https://github.com/apache/incubator-pekko/issues/1141#issuecomment-1970933409

   @JD557 Hi, the related change have been merged, 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: [I] Discussion about restoring statefulMapConcat [incubator-pekko]

Posted by "JD557 (via GitHub)" <gi...@apache.org>.
JD557 commented on issue #1141:
URL: https://github.com/apache/incubator-pekko/issues/1141#issuecomment-1960037460

   For reference, here are some benchmarks (based on the original ZIO Streams benchmarks): https://gist.github.com/JD557/251467aac30860dca88dc3e697dfb381
   
   You can quickly run them with Scala CLI with `scala-cli --power --jmh https://gist.github.com/JD557/251467aac30860dca88dc3e697dfb381`
   
   Here I compare an implementation using `scan`+`mapConcat` (`pekkoScan`), `statefulMapConcat` (`pekkoStatefulMapConcat`) and `statefulMap`+`mapConcat` (pekkoStatefulMapMapConcat)
   
   The tests were run on an Apple M3 Max, using OpenJDK 21.0.2+13-LTS
   
   Results:
   
   ```
   Benchmark                                      (chunkSize)  (cols)  (rows)   Mode  Cnt   Score   Error  Units
   CSVStreamBenchmarks.pekkoScan                         5000     100     100  thrpt    5  27.785 ± 0.080  ops/s
   CSVStreamBenchmarks.pekkoStatefulMapConcat            5000     100     100  thrpt    5  72.419 ± 0.308  ops/s
   CSVStreamBenchmarks.pekkoStatefulMapMapConcat         5000     100     100  thrpt    5  42.094 ± 0.744  ops/s
   ```


-- 
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: [I] Discussion about restoring statefulMapConcat [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin closed issue #1141: Discussion about restoring statefulMapConcat
URL: https://github.com/apache/incubator-pekko/issues/1141


-- 
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