Do not track coroutines with empty coroutine context in DebugProbes (#…

…3784)

Such coroutines typically are a subject for significant debugger overhead, can be observed with more conventional tools, and do not contribute to the state of the system that is typically observed with coroutines debugger

Fixes #3782

Co-authored-by: Oleg Yukhnevich <[email protected]>
Co-authored-by: Dmitry Khalanskiy <[email protected]>

benchmarks/src/jmh/kotlin/benchmarks/debug/DebugProbesConcurrentBenchmark.kt → benchmarks/src/jmh/kotlin/benchmarks/debug/DebugSequenceOverheadBenchmark.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ * Copyright 2016-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
  */
 
 package benchmarks.debug
@@ -9,46 +9,24 @@ import kotlinx.coroutines.debug.*
 import org.openjdk.jmh.annotations.*
 import org.openjdk.jmh.annotations.State
 import java.util.concurrent.*
+import java.util.concurrent.atomic.AtomicInteger
 
+/**
+ * The benchmark is supposed to show the DebugProbes overhead for a non-concurrent sequence builder.
+ * The code is actually part of the IDEA codebase, originally reported here: https://github.com/Kotlin/kotlinx.coroutines/issues/3527
+ */
 @Warmup(iterations = 5, time = 1)
 @Measurement(iterations = 5, time = 1)
 @Fork(value = 1)
 @BenchmarkMode(Mode.AverageTime)
 @OutputTimeUnit(TimeUnit.MICROSECONDS)
 @State(Scope.Benchmark)
-open class DebugProbesConcurrentBenchmark {
-
-    @Setup
-    fun setup() {
-        DebugProbes.sanitizeStackTraces = false
-        DebugProbes.enableCreationStackTraces = false
-        DebugProbes.install()
-    }
-
-    @TearDown
-    fun tearDown() {
-        DebugProbes.uninstall()
-    }
-
+open class DebugSequenceOverheadBenchmark {
 
-    @Benchmark
-    fun run() = runBlocking<Long> {
-        var sum = 0L
-        repeat(8) {
-            launch(Dispatchers.Default) {
-                val seq = stressSequenceBuilder((1..100).asSequence()) {
-                    (1..it).asSequence()
-                }
-
-                for (i in seq) {
-                    sum += i.toLong()
-                }
-            }
-        }
-        sum
-    }
-
-    private fun <Node> stressSequenceBuilder(initialSequence: Sequence<Node>, children: (Node) -> Sequence<Node>): Sequence<Node> {
+    private fun <Node> generateRecursiveSequence(
+        initialSequence: Sequence<Node>,
+        children: (Node) -> Sequence<Node>
+    ): Sequence<Node> {
         return sequence {
             val initialIterator = initialSequence.iterator()
             if (!initialIterator.hasNext()) {
@@ -68,4 +46,45 @@ open class DebugProbesConcurrentBenchmark {
             }
         }
     }
+
+    @Param("true", "false")
+    var withDebugger = false
+
+    @Setup
+    fun setup() {
+        DebugProbes.sanitizeStackTraces = false
+        DebugProbes.enableCreationStackTraces = false
+        if (withDebugger) {
+            DebugProbes.install()
+        }
+    }
+
+    @TearDown
+    fun tearDown() {
+        if (withDebugger) {
+            DebugProbes.uninstall()
+        }
+    }
+
+    // Shows the overhead of sequence builder with debugger enabled
+    @Benchmark
+    fun runSequenceSingleThread(): Int = runBlocking {
+        generateRecursiveSequence((1..100).asSequence()) {
+            (1..it).asSequence()
+        }.sum()
+    }
+
+    // Shows the overhead of sequence builder with debugger enabled and debugger is concurrently stressed out
+    @Benchmark
+    fun runSequenceMultipleThreads(): Int = runBlocking {
+        val result = AtomicInteger(0)
+        repeat(Runtime.getRuntime().availableProcessors()) {
+            launch(Dispatchers.Default) {
+                result.addAndGet(generateRecursiveSequence((1..100).asSequence()) {
+                    (1..it).asSequence()
+                }.sum())
+            }
+        }
+        result.get()
+    }
 }

kotlinx-coroutines-core/api/kotlinx-coroutines-core.api

@@ -938,7 +938,9 @@ public final class kotlinx/coroutines/debug/internal/DebugProbesImpl {
 	public final fun dumpDebuggerInfo ()Ljava/util/List;
 	public final fun enhanceStackTraceWithThreadDump (Lkotlinx/coroutines/debug/internal/DebugCoroutineInfo;Ljava/util/List;)Ljava/util/List;
 	public final fun enhanceStackTraceWithThreadDumpAsJson (Lkotlinx/coroutines/debug/internal/DebugCoroutineInfo;)Ljava/lang/String;
+	public final fun getIgnoreCoroutinesWithEmptyContext ()Z
 	public final fun isInstalled ()Z
+	public final fun setIgnoreCoroutinesWithEmptyContext (Z)V
 }
 
 public final class kotlinx/coroutines/debug/internal/DebugProbesImpl$CoroutineOwner : kotlin/coroutines/Continuation, kotlin/coroutines/jvm/internal/CoroutineStackFrame {

kotlinx-coroutines-core/jvm/src/debug/internal/DebugProbesImpl.kt

@@ -42,6 +42,7 @@ internal object DebugProbesImpl {
 
     internal var sanitizeStackTraces: Boolean = true
     internal var enableCreationStackTraces: Boolean = true
+    public var ignoreCoroutinesWithEmptyContext: Boolean = true
 
     /*
      * Substitute for service loader, DI between core and debug modules.
@@ -422,8 +423,8 @@ internal object DebugProbesImpl {
 
     private fun updateState(frame: Continuation<*>, state: String) {
         if (!isInstalled) return
-        // KT-29997 is here only since 1.3.30
-        if (state == RUNNING && KotlinVersion.CURRENT.isAtLeast(1, 3, 30)) {
+        if (ignoreCoroutinesWithEmptyContext && frame.context === EmptyCoroutineContext) return // See ignoreCoroutinesWithEmptyContext
+        if (state == RUNNING) {
             val stackFrame = frame as? CoroutineStackFrame ?: return
             updateRunningState(stackFrame, state)
             return
@@ -475,6 +476,8 @@ internal object DebugProbesImpl {
     // Not guarded by the lock at all, does not really affect consistency
     internal fun <T> probeCoroutineCreated(completion: Continuation<T>): Continuation<T> {
         if (!isInstalled) return completion
+        // See DebugProbes.ignoreCoroutinesWithEmptyContext for the additional details.
+        if (ignoreCoroutinesWithEmptyContext && completion.context === EmptyCoroutineContext) return completion
         /*
          * If completion already has an owner, it means that we are in scoped coroutine (coroutineScope, withContext etc.),
          * then piggyback on its already existing owner and do not replace completion

kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api

@@ -18,6 +18,7 @@ public final class kotlinx/coroutines/debug/DebugProbes {
 	public static synthetic fun dumpCoroutines$default (Lkotlinx/coroutines/debug/DebugProbes;Ljava/io/PrintStream;ILjava/lang/Object;)V
 	public final fun dumpCoroutinesInfo ()Ljava/util/List;
 	public final fun getEnableCreationStackTraces ()Z
+	public final fun getIgnoreCoroutinesWithEmptyContext ()Z
 	public final fun getSanitizeStackTraces ()Z
 	public final fun install ()V
 	public final fun isInstalled ()Z
@@ -28,6 +29,7 @@ public final class kotlinx/coroutines/debug/DebugProbes {
 	public static synthetic fun printScope$default (Lkotlinx/coroutines/debug/DebugProbes;Lkotlinx/coroutines/CoroutineScope;Ljava/io/PrintStream;ILjava/lang/Object;)V
 	public final fun scopeToString (Lkotlinx/coroutines/CoroutineScope;)Ljava/lang/String;
 	public final fun setEnableCreationStackTraces (Z)V
+	public final fun setIgnoreCoroutinesWithEmptyContext (Z)V
 	public final fun setSanitizeStackTraces (Z)V
 	public final fun uninstall ()V
 	public final fun withDebugProbes (Lkotlin/jvm/functions/Function0;)V

kotlinx-coroutines-debug/src/DebugProbes.kt

@@ -46,6 +46,8 @@ public object DebugProbes {
      * Whether coroutine creation stack traces should be sanitized.
      * Sanitization removes all frames from `kotlinx.coroutines` package except
      * the first one and the last one to simplify diagnostic.
+     *
+     * `true` by default.
      */
     public var sanitizeStackTraces: Boolean
         get() = DebugProbesImpl.sanitizeStackTraces
@@ -59,13 +61,31 @@ public object DebugProbes {
      * thread is captured and attached to the coroutine.
      * This option can be useful during local debug sessions, but is recommended
      * to be disabled in production environments to avoid stack trace dumping overhead.
+     *
+     * `true` by default.
      */
     public var enableCreationStackTraces: Boolean
         get() = DebugProbesImpl.enableCreationStackTraces
         set(value) {
             DebugProbesImpl.enableCreationStackTraces = value
         }
 
+    /**
+     * Whether to ignore coroutines whose context is [EmptyCoroutineContext].
+     *
+     * Coroutines with empty context are considered to be irrelevant for the concurrent coroutines' observability:
+     * - They do not contribute to any concurrent executions
+     * - They do not contribute to the (concurrent) system's liveness and/or deadlocks, as no other coroutines might wait for them
+     * - The typical usage of such coroutines is a combinator/builder/lookahead parser that can be debugged using more convenient tools.
+     *
+     * `true` by default.
+     */
+    public var ignoreCoroutinesWithEmptyContext: Boolean
+        get() = DebugProbesImpl.ignoreCoroutinesWithEmptyContext
+        set(value) {
+            DebugProbesImpl.ignoreCoroutinesWithEmptyContext = value
+        }
+
     /**
      * Determines whether debug probes were [installed][DebugProbes.install].
      */
@@ -122,13 +142,14 @@ public object DebugProbes {
      * Throws [IllegalStateException] if the scope has no a job in it.
      */
     public fun printScope(scope: CoroutineScope, out: PrintStream = System.out): Unit =
-       printJob(scope.coroutineContext[Job] ?: error("Job is not present in the scope"), out)
+        printJob(scope.coroutineContext[Job] ?: error("Job is not present in the scope"), out)
 
     /**
      * Returns all existing coroutines' info.
      * The resulting collection represents a consistent snapshot of all existing coroutines at the moment of invocation.
      */
-    public fun dumpCoroutinesInfo(): List<CoroutineInfo> = DebugProbesImpl.dumpCoroutinesInfo().map { CoroutineInfo(it) }
+    public fun dumpCoroutinesInfo(): List<CoroutineInfo> =
+        DebugProbesImpl.dumpCoroutinesInfo().map { CoroutineInfo(it) }
 
     /**
      * Dumps all active coroutines into the given output stream, providing a consistent snapshot of all existing coroutines at the moment of invocation.

kotlinx-coroutines-debug/test/StandardBuildersDebugTest.kt

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2016-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+package kotlinx.coroutines.debug
+
+import org.junit.Test
+import kotlin.test.*
+
+class StandardBuildersDebugTest : DebugTestBase() {
+
+    @Test
+    fun testBuildersAreMissingFromDumpByDefault() = runTest {
+        val (b1, b2) = createBuilders()
+
+        val coroutines = DebugProbes.dumpCoroutinesInfo()
+        assertEquals(1, coroutines.size)
+        assertTrue { b1.hasNext() && b2.hasNext() } // Don't let GC collect our coroutines until the test is complete
+    }
+
+    @Test
+    fun testBuildersCanBeEnabled() = runTest {
+        try {
+            DebugProbes.ignoreCoroutinesWithEmptyContext = false
+            val (b1, b2) = createBuilders()
+            val coroutines = DebugProbes.dumpCoroutinesInfo()
+            assertEquals(3, coroutines.size)
+            assertTrue { b1.hasNext() && b2.hasNext() } // Don't let GC collect our coroutines until the test is complete
+        } finally {
+            DebugProbes.ignoreCoroutinesWithEmptyContext = true
+        }
+    }
+
+    private fun createBuilders(): Pair<Iterator<Int>, Iterator<Int>> {
+        val fromSequence = sequence {
+            while (true) {
+                yield(1)
+            }
+        }.iterator()
+
+        val fromIterator = iterator {
+            while (true) {
+                yield(1)
+            }
+        }
+        // Start coroutines
+        fromIterator.hasNext()
+        fromSequence.hasNext()
+        return fromSequence to fromIterator
+    }
+}