kotlinx.coroutines icon indicating copy to clipboard operation
kotlinx.coroutines copied to clipboard

Decide on each particular machinery of debug mode and provide an additional feature flag for each

Open qwwdfsad opened this issue 2 years ago • 5 comments

Currently, when coroutines debug mode is enabled, the following machinery is enabled:

  • Every coroutine created is supplied with CoroutineId. This one might be useful for local debugging and logging, though its general applicability (i.e. for running applications of IDEA scale, the purpose of numeric id is unclear)
  • Stacktrace population for JobCancellationException is enabled, which is known to be a performance-hitter (we also know it from IDEA experience)
  • Thread.setName with coroutine id is invoked. This seemed to be the legacy of earlier days when we had little to no tooling around coroutines debugging. It has three downsides: it crashes ART consistently (#2234), it might dominate coroutine-specific workloads (https://github.com/Kotlin/kotlinx.coroutines/issues/2234#issuecomment-1204922620), and it potentially interferes with code that leverages thread names.
  • Stacktrace recovery is enabled. I like this one and have no intention of tweaking it :)

qwwdfsad avatar Mar 13 '23 17:03 qwwdfsad

My very first proposal is to completely get rid of setName -- it seems to cause more harm than good.

If you have any additional input about CancellationException stacktraces (or their absence) or CoroutineId -- please feel free to share, whether you are actively using them or, contrary, see no value in it.

qwwdfsad avatar Mar 13 '23 17:03 qwwdfsad

Also, these feature flags should have a clear priority over debug mode -- it should be possible to e.g. enable stacktrace recovery without enabling debug mode itself for more fine-grained tuning

qwwdfsad avatar Mar 13 '23 17:03 qwwdfsad

In IJ there is little to zero value in CoroutineId; we advise to use CoroutineName explicitly instead

dovchinnikov avatar Jul 03 '23 15:07 dovchinnikov

UDP: Fleet disables stacktrace recovery because of setName overhead, but would like to enable it back

qwwdfsad avatar Nov 03 '23 14:11 qwwdfsad

I'd also like to request a switch to include CoroutineName to toString representation of AbstractCoroutine (CoroutineId is not needed). At the moment our coroutine dump has to do it manually, and it does not help when it's not clear which coroutine was cancelled when JobCancellationException is printed to logs.

dovchinnikov avatar Nov 03 '23 14:11 dovchinnikov