Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kamon-pekko: find new way to instrument pinned dispatchers #1366

Open
pjfanning opened this issue Sep 19, 2024 · 2 comments
Open

kamon-pekko: find new way to instrument pinned dispatchers #1366

pjfanning opened this issue Sep 19, 2024 · 2 comments

Comments

@pjfanning
Copy link
Contributor

Relates to #1352

The existing ThreadPoolConfigCopyAdvice kamon-pekko doesn't work for Pekko 1.1 when Scala 2.13 is used. Pekko 1.1 has Scala 2 compiler inlining enabled. ThreadPoolConfig is a final case class and the copy function is derived by the Scala compiler. When a class is final, all its functions are final and can be inlined. Since the copy function is not explicitly declared, I haven't found a way to mark it as @noinline. I even tried adding the annotation to the case class.

  onType("org.apache.pekko.dispatch.ThreadPoolConfig")
    .mixin(classOf[HasDispatcherPrerequisites.Mixin])
    .advise(method("copy"), classOf[ThreadPoolConfigCopyAdvice])

Would it be feasible to try replacing this instrumentation point and concentrating on the PinnedDispatcher constructor instead?
https://github.com/apache/pekko/blob/22a738511375baa23c5b84c87d44a065c0401e0e/actor/src/main/scala/org/apache/pekko/dispatch/PinnedDispatcher.scala#L28

fyi @hughsimpson

@pjfanning
Copy link
Contributor Author

I guess where I went wrong is that I assumed you needed to do an override def if you wanted to replace the copy function but apache/pekko#1489 appears to show that you don't need the override. The override causes compilation issues.

@hughsimpson
Copy link
Contributor

@pjfanning Had just finished getting the test suite to pass when I saw this message 🤦 Indeed the solution I went for was to override the copy method, although the other solution seems to be instrumenting every copy callsite with @noinline which is... even less elegant, and anyway didn't seem to work for me. It would probably be better to try something else, as you say...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants