Skip to content

A way to statically load DebugProbesKt in production #3633

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

Open
dovchinnikov opened this issue Feb 21, 2023 · 3 comments
Open

A way to statically load DebugProbesKt in production #3633

dovchinnikov opened this issue Feb 21, 2023 · 3 comments
Assignees
Labels

Comments

@dovchinnikov
Copy link
Contributor

dovchinnikov commented Feb 21, 2023

-javaagent is not a production-level solution for IJ, so IJ enables coroutine debugger dynamically at very early stage of application start up:

DebugProbes.enableCreationStackTraces = false
DebugProbes.install()

What do we have now?

  • kotlinx-coroutines-debug bundles repackaged byte-buddy (15,3 MB out of total 20,3 MB).
  • Considerable time is spent on loading the byte-buddy (anywhere from 300ms to 500ms in our runs).
  • It's used only to patch a single class kotlin.coroutines.jvm.internal.DebugProbesKt which is located in kotlin-stdlib.
  • It does not work on winarm64 at the moment (minor).

What should be instead?

A way to statically load the proper binary without dealing with byte-buddy.

Why?

  • Reduce the size of production jars.
  • Reduce the class loading time.

Possible solution

Replace stdlib's kotlin.coroutines.jvm.internal.DebugProbesKt with the following:

private val probesBridge get() = Class.forName("ProbesBridge Name TBD")
private val probeCoroutineCreatedMethod: Method? = runCatching {
  probesBridge.getDeclaredMethod("probeCoroutineCreated", Continuation::class.java)
}.getOrNull()
private val probeCoroutineResumedMethod: Method? = runCatching {
  probesBridge.getDeclaredMethod("probeCoroutineResumed", Continuation::class.java)
}.getOrNull()
private val probeCoroutineSuspended: Method? = runCatching {
  probesBridge.getDeclaredMethod("probeCoroutineSuspended", Continuation::class.java)
}.getOrNull()

internal fun <T> probeCoroutineCreated(completion: Continuation<T>): Continuation<T> {
  if (probeCoroutineCreatedMethod != null) {
    return probeCoroutineCreatedMethod.invoke(null, completion) as Continuation<T>
  }
  return completion
}

internal fun probeCoroutineResumed(frame: Continuation<*>) {
  probeCoroutineResumedMethod?.invoke(null, frame)
}

internal fun probeCoroutineSuspended(frame: Continuation<*>) {
  probeCoroutineSuspended?.invoke(null, frame)
}

This approach is already used here.
JIT should not have any trouble inlining one of == null branches when ProbesBridge is present or absent.

  1. ProbesBridge should set AgentInstallationType in its <clinit>.
  2. ProbesBridge should start the cleaner thread in its <clinit> (questionable, but enables no-code setup by putting the debug jar into classpath).
  3. (Optionally) Provide a separate minimal production-ready kotlinx-coroutines-debug-core JAR with probes.
    kotlinx.coroutines.debug.internal.DebugProbesImpl resides in kotlinx-coroutines-core-jvm, I think that it should be a part of that JAR instead.

Other solutions

  1. Patch kotlin-stdlib jar statically, replace kotlin.coroutines.jvm.internal.DebugProbesKt during build time, publish a separate kotlin-stdlib artefact.
  2. Patch kotlin-stdlib jar dynamically.
    2.1. Put the jar before kotlin-stdlib in classpath, similar to Please provide artifact with DebugProbes for classpath patching instead of instrumentations. #3356.
    2.2. Change IJ own class loader to load DebugProbesKt.bin or the proposed class when kotlin.coroutines.jvm.internal.DebugProbesKt is requested (somewhat equivalent to javaagent solution, also a hack).
  3. DI probes into stdlib.
    3.1. proposed solution, or
    3.2. separate interface,ServiceLoader similar to Dispatchers.Main, no op implementation by default.
@qwwdfsad
Copy link
Collaborator

-javaagent is not a production-level solution for IJ

From our internal notes:

  • Passing -javaagent everywhere in each launch script is quite a lot of technical burden
  • It potentially (?) affects classloading times as it processes all classes in IJ

@sellmair
Copy link
Member

sellmair commented Oct 1, 2024

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

No branches or pull requests

3 participants