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 2023/12/23 13:04:34 UTC
[PR] =str Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
He-Pin opened a new pull request, #864:
URL: https://github.com/apache/incubator-pekko/pull/864
Motivation:
There is a allocation of `Some` in `SupervisedGraphStageLogic` and for performance reason, I want to remove that.
I can choose the `Option[T]` to `OptionVal[T]`, but that will cause the problem if the user specified function return `null`.
So I choose to drop the usage of `SupervisedGraphStageLogic`.
Result:
Reduce allocation.
Status: Mima file is not added yet, open for disccusion.
--
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] =str Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868308442
I have reviewed some of your PRs, but due to my lack of familiarity with the project, I did not comment. But I noticed that there is a '=str' in the title all of them, and I don't quite understand what it means.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868943256
@laglangyue it's simple Java/Kotlin like code, try catch thing:)
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436522904
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,26 +178,31 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
- case Some(flag) =>
- if (flag) pull(in)
- else {
- push(out, elem)
- setHandler(in, rest)
+ new GraphStageLogic(shape) with InHandler with OutHandler {
+ private lazy val decider = inheritedAttributes.mandatoryAttribute[SupervisionStrategy].decider
+
+ override def onPush(): Unit =
+ try {
+ val elem = grab(in)
+ if (p(elem)) {
+ pull(in)
+ } else {
+ push(out, elem)
+ setHandler(in, rest)
+ }
+ } catch {
+ case NonFatal(ex) =>
+ decider(ex) match {
+ case Supervision.Stop => failStage(ex)
+ case Supervision.Resume => if (!hasBeenPulled(in)) pull(in)
+ case Supervision.Restart => if (!hasBeenPulled(in)) pull(in)
Review Comment:
case _,or is it has a default case option
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1869691708
Thank you both from China Scala community to review this, I was plan to add onErrorReturn and OnErrorResume and OnErrorComplete operators, which need very handy control around the supervision.
--
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] =str Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868330363
> > I have reviewed some of your PRs, but due to my lack of familiarity with the project, I did not comment. But I noticed that there is a '=str' in the title all of them, and I don't quite understand what it means.
>
> I was using the old style, maybe chore nowadays ?
=means I added no api change, and str is abbr for stream module.
--
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] =str Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868383464
Im with @pjfanning here, I would rather drop this convention (i.e. `=str`) because no one else is doing it so it kind of defeats the point.
For me, simple git commit messages with active tense as well as making sure commits are atomic as possible is whats most important, i.e. general guidelines as from https://reflectoring.io/meaningful-commit-messages/
--
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] =str Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868384002
> but that will cause the problem if the user specified function return `null`.
Is this a legitimate issue, I mean we can do behavioural changes if its well justified?
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868442665
> I was using the old style, maybe chore nowadays ?
> =means I added no api change, and str is abbr for stream module. BTW, I see you from China, are you in the wechat group ？
yeah, I am in Hangzhou,Zhejiang, and I am newcommner for pekko/akka，I plan to introduce Pekko into my project in future.
it's my wechatId `laglangyue`, could you invite me to join the wechat group.
I am not picky about style, and just a simple question.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1870875897
> Too much duplicate code is not good to maintenance and code readability.
I wouldn't be surprised if the inliner (i.e. https://github.com/apache/incubator-pekko/pull/857) will automatically do some of these performance improvements, unfortunately it would only work for Scala 2
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436723501
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,26 +178,31 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
Review Comment:
remove the method `withSupervision`, and add a `try catch`
can we concise the implement
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436765905
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,25 +178,26 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
- case Some(flag) =>
- if (flag) pull(in)
- else {
- push(out, elem)
- setHandler(in, rest)
+ new GraphStageLogic(shape) with InHandler with OutHandler {
+ private lazy val decider = inheritedAttributes.mandatoryAttribute[SupervisionStrategy].decider
+
+ override def onPush(): Unit =
+ try {
+ val elem = grab(in)
+ if (p(elem)) {
+ pull(in)
+ } else {
+ push(out, elem)
+ setHandler(in, () => push(out, grab(in)))
Review Comment:
One last complain, i think this line lambda will generate anonymous class, it is hard to identify on profiler tools.
But i will still approve this change, because i think this method won't be top of hotspot tree and the original code use the same 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: [PR] =str Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868330218
> I have reviewed some of your PRs, but due to my lack of familiarity with the project, I did not comment. But I noticed that there is a '=str' in the title all of them, and I don't quite understand what it means.
I was using the old style, maybe chore nowadays ?
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436775684
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,25 +178,26 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
- case Some(flag) =>
- if (flag) pull(in)
- else {
- push(out, elem)
- setHandler(in, rest)
+ new GraphStageLogic(shape) with InHandler with OutHandler {
+ private lazy val decider = inheritedAttributes.mandatoryAttribute[SupervisionStrategy].decider
+
+ override def onPush(): Unit =
+ try {
+ val elem = grab(in)
+ if (p(elem)) {
+ pull(in)
+ } else {
+ push(out, elem)
+ setHandler(in, () => push(out, grab(in)))
Review Comment:
That's true, was a anonymous class and now a lambda$, I think this should be fine as there are other cases where a lambda is been used,eg:`emit`.
--
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] =str Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868430813
> > but that will cause the problem if the user specified function return `null`.
>
> Is this a legitimate issue, I mean we can do behavioural changes if its well justified?
I think we can't, if I change to make use of OptionVal, then it will just ignore the value and do nothing, which was a Some(null) and throw an NPE.
and I will update the commit message then.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868731432
I don't have a deep understanding of akka/pekko, so I can't comment on this PR.
> @laglangyue Are you using Pekko at work too, welcome and you are welcome to review this PR too.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1868445745
@laglangyue Are you using Pekko at work too, welcome and you are welcome to review this PR too.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436724228
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,26 +178,31 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
Review Comment:
I was plan to remove this `SupervisedGraphStageLogic`, this call class will add at least 3 stack frame too.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436765905
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,25 +178,26 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
- case Some(flag) =>
- if (flag) pull(in)
- else {
- push(out, elem)
- setHandler(in, rest)
+ new GraphStageLogic(shape) with InHandler with OutHandler {
+ private lazy val decider = inheritedAttributes.mandatoryAttribute[SupervisionStrategy].decider
+
+ override def onPush(): Unit =
+ try {
+ val elem = grab(in)
+ if (p(elem)) {
+ pull(in)
+ } else {
+ push(out, elem)
+ setHandler(in, () => push(out, grab(in)))
Review Comment:
One last complain, i think this line lambda will generate anonymous class, it is hard to identify on profiler tools.
But i will still approve this change, because i think this method won't be top of hotspot tree.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#issuecomment-1870796470
@mdedetrich @pjfanning ping~
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "Roiocam (via GitHub)" <gi...@apache.org>.
Roiocam commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436533416
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,26 +178,31 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
- case Some(flag) =>
- if (flag) pull(in)
- else {
- push(out, elem)
- setHandler(in, rest)
+ new GraphStageLogic(shape) with InHandler with OutHandler {
+ private lazy val decider = inheritedAttributes.mandatoryAttribute[SupervisionStrategy].decider
+
+ override def onPush(): Unit =
+ try {
+ val elem = grab(in)
+ if (p(elem)) {
+ pull(in)
+ } else {
+ push(out, elem)
+ setHandler(in, rest)
+ }
+ } catch {
+ case NonFatal(ex) =>
+ decider(ex) match {
+ case Supervision.Stop => failStage(ex)
+ case Supervision.Resume => if (!hasBeenPulled(in)) pull(in)
+ case Supervision.Restart => if (!hasBeenPulled(in)) pull(in)
}
- case None => // do nothing
}
- }
- def rest = new InHandler {
- def onPush() = push(out, grab(in))
+ private def rest: InHandler = new InHandler {
Review Comment:
Maybe we should keep the code simple and let the compiler do more work, isn't it?
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436728259
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,26 +178,31 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
- case Some(flag) =>
- if (flag) pull(in)
- else {
- push(out, elem)
- setHandler(in, rest)
+ new GraphStageLogic(shape) with InHandler with OutHandler {
+ private lazy val decider = inheritedAttributes.mandatoryAttribute[SupervisionStrategy].decider
+
+ override def onPush(): Unit =
+ try {
+ val elem = grab(in)
+ if (p(elem)) {
+ pull(in)
+ } else {
+ push(out, elem)
+ setHandler(in, rest)
+ }
+ } catch {
+ case NonFatal(ex) =>
+ decider(ex) match {
+ case Supervision.Stop => failStage(ex)
+ case Supervision.Resume => if (!hasBeenPulled(in)) pull(in)
+ case Supervision.Restart => if (!hasBeenPulled(in)) pull(in)
}
- case None => // do nothing
}
- }
- def rest = new InHandler {
- def onPush() = push(out, grab(in))
+ private def rest: InHandler = new InHandler {
Review Comment:
Good, I was want to avoid scala 2.12 bug.
--
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] perf: Drop the usages of SupervisedGraphStageLogic to reduce allocation. [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #864:
URL: https://github.com/apache/incubator-pekko/pull/864#discussion_r1436726965
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/Ops.scala:
##########
@@ -178,26 +178,31 @@ import pekko.util.ccompat._
override def initialAttributes: Attributes = DefaultAttributes.dropWhile and SourceLocation.forLambda(p)
def createLogic(inheritedAttributes: Attributes) =
- new SupervisedGraphStageLogic(inheritedAttributes, shape) with InHandler with OutHandler {
- override def onPush(): Unit = {
- val elem = grab(in)
- withSupervision(() => p(elem)) match {
- case Some(flag) =>
- if (flag) pull(in)
- else {
- push(out, elem)
- setHandler(in, rest)
+ new GraphStageLogic(shape) with InHandler with OutHandler {
+ private lazy val decider = inheritedAttributes.mandatoryAttribute[SupervisionStrategy].decider
+
+ override def onPush(): Unit =
+ try {
+ val elem = grab(in)
+ if (p(elem)) {
+ pull(in)
+ } else {
+ push(out, elem)
+ setHandler(in, rest)
+ }
+ } catch {
+ case NonFatal(ex) =>
+ decider(ex) match {
+ case Supervision.Stop => failStage(ex)
+ case Supervision.Resume => if (!hasBeenPulled(in)) pull(in)
+ case Supervision.Restart => if (!hasBeenPulled(in)) pull(in)
Review Comment:
--
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