You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "mdedetrich (via GitHub)" <gi...@apache.org> on 2023/12/30 04:48:47 UTC

[PR] Fix remaining missing inline statements/annotations [incubator-pekko]

mdedetrich opened a new pull request, #889:
URL: https://github.com/apache/incubator-pekko/pull/889

   The previous PR at https://github.com/apache/incubator-pekko/pull/857 managed to miss some inline annotations so this PR fills the rest.
   
   Interestingly I managed to find a Scala 3 bug whereby if you mark a method with `inline` it cannot be called within Java sources because its missing a symbol, I have reported the issue upstream at https://github.com/lampepfl/dotty/issues/19346


-- 
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] Fix remaining missing inline statements/annotations [incubator-pekko]

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich merged PR #889:
URL: https://github.com/apache/incubator-pekko/pull/889


-- 
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] Fix remaining missing inline statements/annotations [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #889:
URL: https://github.com/apache/incubator-pekko/pull/889#discussion_r1438473462


##########
actor/src/main/scala-2.13/org/apache/pekko/dispatch/internal/SameThreadExecutionContext.scala:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+/*
+ * Copyright (C) 2009-2022 Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package org.apache.pekko.dispatch.internal
+
+import scala.concurrent.ExecutionContext
+
+import org.apache.pekko
+import pekko.annotation.InternalApi
+
+/**
+ * Factory to create same thread ec. Not intended to be called from any other site than to create [[pekko.dispatch.ExecutionContexts#parasitic]]
+ *
+ * INTERNAL API
+ */
+@InternalApi
+private[dispatch] object SameThreadExecutionContext {
+  @inline def apply(): ExecutionContext = ExecutionContext.parasitic

Review Comment:
   Do we really need this SameThreadExecutionContext?



-- 
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] Fix remaining missing inline statements/annotations [incubator-pekko]

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #889:
URL: https://github.com/apache/incubator-pekko/pull/889#issuecomment-1872463638

   Im going to go ahead and merge this now so I can fix any potential future issues in the nightly runs on the other pekko modules.


-- 
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] Fix remaining missing inline statements/annotations [incubator-pekko]

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #889:
URL: https://github.com/apache/incubator-pekko/pull/889#discussion_r1438473727


##########
actor/src/main/scala-2.13/org/apache/pekko/dispatch/internal/SameThreadExecutionContext.scala:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+/*
+ * Copyright (C) 2009-2022 Lightbend Inc. <https://www.lightbend.com>
+ */
+
+package org.apache.pekko.dispatch.internal
+
+import scala.concurrent.ExecutionContext
+
+import org.apache.pekko
+import pekko.annotation.InternalApi
+
+/**
+ * Factory to create same thread ec. Not intended to be called from any other site than to create [[pekko.dispatch.ExecutionContexts#parasitic]]
+ *
+ * INTERNAL API
+ */
+@InternalApi
+private[dispatch] object SameThreadExecutionContext {
+  @inline def apply(): ExecutionContext = ExecutionContext.parasitic

Review Comment:
   Yes, see https://github.com/apache/incubator-pekko/blob/b0fdac259bd57fdd481483f3fe9a7aec6e1ff38a/actor/src/main/scala-2.12/org/apache/pekko/dispatch/internal/SameThreadExecutionContext.scala for why



-- 
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] Fix remaining missing inline statements/annotations [incubator-pekko]

Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #889:
URL: https://github.com/apache/incubator-pekko/pull/889#issuecomment-1872455608

   > For method which would be called from Java, maybe was expected be marked with noinline?
   
   Maybe but its also not intuitive behaviour. For starters nothing is preventing you from calling the method in Scala (in which case we want it to be inlined) and its also different from Scala 2's `@inline` annotation which can still be called as a standard method in Java.
   
   `@noinline` is different, its when you want to forcefully prevent any inlining at all and at least in Pekko its used in cases where the stack trace is being inspected


-- 
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] Fix remaining missing inline statements/annotations [incubator-pekko]

Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #889:
URL: https://github.com/apache/incubator-pekko/pull/889#issuecomment-1872454726

   For method which would be called from Java, maybe was expected be marked with noinline?


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