Skip to content

Commit df05db6

Browse files
committed
~fast path for undispatched
1 parent ec3ba2f commit df05db6

File tree

2 files changed

+38
-28
lines changed

2 files changed

+38
-28
lines changed

kotlinx-coroutines-core/jvm/src/CoroutineContext.kt

+37-27
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package kotlinx.coroutines
77
import kotlinx.coroutines.internal.*
88
import kotlinx.coroutines.scheduling.*
99
import kotlin.coroutines.*
10-
import kotlin.coroutines.jvm.internal.*
1110
import kotlin.coroutines.jvm.internal.CoroutineStackFrame
1211

1312
internal const val COROUTINES_SCHEDULER_PROPERTY_NAME = "kotlinx.coroutines.scheduler"
@@ -57,7 +56,7 @@ internal actual inline fun <T> withContinuationContext(continuation: Continuatio
5756
val oldValue = updateThreadContext(context, countOrElement)
5857
val undispatchedCompletion = if (oldValue !== NO_THREAD_ELEMENTS) {
5958
// Only if some values were replaced we'll go to the slow path of figuring out where/how to restore them
60-
updateUndispatchedCompletion(context, oldValue)
59+
continuation.updateUndispatchedCompletion(context, oldValue)
6160
} else {
6261
null // fast path -- don't even try to find undispatchedCompletion as there's nothing to restore in the context
6362
}
@@ -70,44 +69,55 @@ internal actual inline fun <T> withContinuationContext(continuation: Continuatio
7069
}
7170
}
7271

73-
internal fun updateUndispatchedCompletion(context: CoroutineContext, oldValue: Any?): UndispatchedCoroutine<*>? {
74-
val undispatched = context[UndispatchedMarker]?.coroutine ?: return null
75-
undispatched.saveThreadContext(context, oldValue)
76-
return undispatched
72+
internal fun Continuation<*>.updateUndispatchedCompletion(context: CoroutineContext, oldValue: Any?): UndispatchedCoroutine<*>? {
73+
if (this !is CoroutineStackFrame) return null
74+
/*
75+
* Fast-path to detect whether we have unispatched coroutine at all in our stack.
76+
*
77+
* Implementation note.
78+
* If we ever find that stackwalking for thread-locals is way too slow, here is another idea:
79+
* 1) Store undispatched coroutine right in the `UndispatchedMarker` instance
80+
* 2) To avoid issues with cross-dispatch boundary, remove `UndispatchedMarker`
81+
* from the context when creating dispatched coroutine in `withContext`.
82+
* Another option is to "unmark it" instead of removing to save an allocation.
83+
* Both options should work, but it requires more careful studying of the performance
84+
* and, mostly, maintainability impact.
85+
*/
86+
val potentiallyHasUndispatchedCorotuine = context[UndispatchedMarker] !== null
87+
if (!potentiallyHasUndispatchedCorotuine) return null
88+
val completion = undispatchedCompletion()
89+
completion?.saveThreadContext(context, oldValue)
90+
return completion
91+
}
92+
93+
internal tailrec fun CoroutineStackFrame.undispatchedCompletion(): UndispatchedCoroutine<*>? {
94+
// Find direct completion of this continuation
95+
val completion: CoroutineStackFrame = when (this) {
96+
is DispatchedCoroutine<*> -> return null
97+
else -> callerFrame ?: return null // something else -- not supported
98+
}
99+
if (completion is UndispatchedCoroutine<*>) return completion // found UndispatchedCoroutine!
100+
return completion.undispatchedCompletion() // walk up the call stack with tail call
77101
}
78102

79103
/**
80-
* Undispatched marker required to properly locate undispatched switch points and restore thread-local context to its initial value.
81-
* Is not added as is as job to avoid being overridden.
82-
*
83-
* `var` is required to avoid cyclic initialization problem with `this`. It's late-binded once in the
84-
* [UndispatchedCoroutine] ctor.
104+
* Marker indicating that [UndispatchedCoroutine] exists somewhere up in the stack.
105+
* Used as a performance optimization to avoid stack walking where it is not nesessary.
85106
*/
86-
internal class UndispatchedMarker(
87-
@JvmField var coroutine: UndispatchedCoroutine<*>?
88-
) : AbstractCoroutineContextElement(Key) {
89-
companion object Key : CoroutineContext.Key<UndispatchedMarker>
107+
private object UndispatchedMarker: CoroutineContext.Element, CoroutineContext.Key<UndispatchedMarker> {
108+
override val key: CoroutineContext.Key<*>
109+
get() = this
90110
}
91111

92112
// Used by withContext when context changes, but dispatcher stays the same
93-
internal actual class UndispatchedCoroutine<in T>(
113+
internal actual class UndispatchedCoroutine<in T>actual constructor (
94114
context: CoroutineContext,
95-
marker: UndispatchedMarker,
96115
uCont: Continuation<T>
97-
) : ScopeCoroutine<T>(context + marker, uCont) {
98-
99-
actual constructor(
100-
context: CoroutineContext,
101-
uCont: Continuation<T>
102-
) : this(context, UndispatchedMarker(null), uCont)
116+
) : ScopeCoroutine<T>(context + UndispatchedMarker, uCont) {
103117

104118
private var savedContext: CoroutineContext? = null
105119
private var savedOldValue: Any? = null
106120

107-
init {
108-
marker.coroutine = this
109-
}
110-
111121
fun saveThreadContext(context: CoroutineContext, oldValue: Any?) {
112122
savedContext = context
113123
savedOldValue = oldValue

kotlinx-coroutines-core/jvm/test/ThreadContextElementRestoreTest.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,4 @@ class ThreadContextElementRestoreTest : TestBase() {
195195
yield()
196196
}
197197
}
198-
}
198+
}

0 commit comments

Comments
 (0)