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

Coroutine cancellation calls Class#getSimpleName on JVM

Open louispf opened this issue 2 years ago • 4 comments

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

louispf avatar Sep 26 '23 16:09 louispf

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)

qwwdfsad avatar Oct 03 '23 10:10 qwwdfsad

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

qwwdfsad avatar Oct 03 '23 12:10 qwwdfsad

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

qwwdfsad avatar Oct 05 '23 09:10 qwwdfsad

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.

louispf avatar Oct 05 '23 14:10 louispf