mark stop() method as noinline

[pjfanning]

relates to #1484

I have similar issues in https://github.com/pjfanning/micrometer-pekko and this change fixes the broken test I ran into there when upgrading to Pekko 1.1.

I have tested this locally and it fixes the issue in micrometer-pekko.

More changes will be needed for Kamon but I'm raising this to show a path forward and to see if this acceptable for Pekko 1.1.2. We can add more changes in more PRs.

I won't merge this until a few people have reviewed it and there are no objections.

fyi @hughsimpson

[Roiocam]

I wonder why this method has been inline. Does us toggle something on? If does, for those method like start, resume, suspend shouldn't be inline too.

[jrudolph]

The Scala compiler is free to inline methods when it is sound. This is the case, when the final receiver of a call can be determined. Whether or not that has any performance impact is always hard to say. In general, the JVM JIT compilation will also do heavy inlining and may in many cases do the same kind of optimizations at runtime. In other cases, like e.g. for higher-order methods, the Scala compiler might be able to generate significantly less code by earlier inlining, which also helps JIT compilations.

That said, the point of adding @noinline here is that some telemetry tools like Kamon instrument internal APIs to do their work. This has mainly historic reasons. In the future, it could be better to introduce official hooks for these kinds of use cases, so that it is easier to track what the public telemetry API is instead of relying on keeping some internal APIs stable.

[Roiocam]

Thanks for the clarification. @jrudolph

[Roiocam]

I am not sure this PR can be resolve the kamon issue, because Kamon Instruementation seem like didn't advise/intercept this method.

the stack trace of the original issue post shows that the DispatcherPrerequisites weren't assigned correctly.

Is it probably caused by this inline?

pekko/actor/src/main/scala-2.13/org/apache/pekko/compat/PartialFunction.scala

Lines 28 to 29 in 0f1db53

	@inline def fromFunction[A, B](f: A => B): scala.PartialFunction[A, B] =
	scala.PartialFunction.fromFunction(f)

[pjfanning]

Thanks @Roiocam - there will need to be more changes. This PR is just to show one change and to highlight that the change doesn't break anything (tests or binary compatibility checks).

[pjfanning]

#1489 adds more annotations

[jrudolph]

It would be good to document these unwritten compatibility constraints. In Akka, we used @InternalStableApi but any kind of simple documentation will help to later understand the reasoning behind adding something like @noinline (which otherwise is very uncommon in Scala and has potential performance impacts in the future).

[pjfanning]

It would be good to document these unwritten compatibility constraints. In Akka, we used @InternalStableApi but any kind of simple documentation will help to later understand the reasoning behind adding something like @noinline (which otherwise is very uncommon in Scala and has potential performance impacts in the future).

@jrudolph I've summarised the situation in intro to #1487 and the aim of that discussion is to discuss how we can remove the @noinline annotations at some point in the future.

[pjfanning]

#1489 was merged - would it be possible for any Pekko PMC members to review this? This change is more useful for micrometer-pekko than Kamon.

[nvollmar]

lgtm