kotlinx.coroutines
kotlinx.coroutines copied to clipboard
Coroutine cancellation calls Class#getSimpleName on JVM
I was doing some trace analysis for an Android application and I noticed that CoroutineScope#cancel was taking up a very large part of this particular microbenchmark run (1/3rd of total allocations, 40% of time) - in particular one obvious thing seems to be the use of Class#getSimpleName, which comes from:
https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/common/src/JobSupport.kt#L416
Which uses:
https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/jvm/src/DebugStrings.kt#L21
Would it be possible to remove this / replace it with something cheaper? There are some other areas for improvement here I think (some of this is tracked in https://github.com/Kotlin/kotlinx.coroutines/issues/3887), but this looks like a simpler task
Could you please share your microbenchmark?
Would it be possible to remove this / replace it with something cheaper?
Probably yes, I'll investigate. Would be nice to know if this code path is visible on a dedicated microbenchmark or during regular usages (e.g. in a microbenchmark that does not stress cancellation mechanism)
I also would like to know what the root cause of the issue is.
From my experiments, ::class.java.simpleName is roughly equivalent to the virtual method call -- so it's not much we can save except without losing the debuggability of the exception message
The root cause is that Android implementation leverages the pre-Java 11 version of getSimpleName (https://github.com/openjdk/jdk8u-dev/blob/master/jdk/src/share/classes/java/lang/Class.java#L1305) that is not only not-cached but quite heavy as well.
We might want to fix that on Android.
Reproducing benchmark from Android folks:
val flow = MutableSharedFlow<Boolean>(
extraBufferCapacity = 16,
onBufferOverflow = BufferOverflow.DROP_OLDEST,
)
coroutineScope.launch {
flow.collect {}
}
// Cancel scope in the benchmark itself
I filed: https://issuetracker.google.com/issues/303631821 to track this issue from Android side. The general recommendation though is for apps / libraries to handle caching themselves, as they know more about the context / can be more efficient with targeted caching. Even if changes were made from the Android side, they would only impact newer devices, so we would still want to be more efficient here for older devices.