Skip to content

Commit 9c68b61

Browse files
committed
~ Don't do extra help in conflated channels
* conflatePreviousSendBuffered does not need to help when its own node was removed. * LockFreeLinkedList.helpRemovePrev does not need to look for the next non-removed node to ensure progress.
1 parent 92f1c8e commit 9c68b61

File tree

5 files changed

+19
-19
lines changed

5 files changed

+19
-19
lines changed

kotlinx-coroutines-core/common/src/channels/ConflatedChannel.kt

+5-4
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
package kotlinx.coroutines.channels
66

7-
import kotlinx.coroutines.selects.*
87
import kotlinx.coroutines.internal.*
8+
import kotlinx.coroutines.selects.*
99

1010
/**
1111
* Channel that buffers at most one element and conflates all subsequent `send` and `offer` invocations,
@@ -46,13 +46,14 @@ internal open class ConflatedChannel<E> : AbstractChannel<E>() {
4646
}
4747

4848
private fun conflatePreviousSendBuffered(node: SendBuffered<E>) {
49-
// Conflate all previous SendBuffered, helping other sends to conflate
50-
var prev = node.prevNode
49+
// Conflate all previous SendBuffered, helping other sends to conflate, but only as long
50+
// as this node itself is not removed
51+
var prev = node.prevNodeIfNotRemoved ?: return
5152
while (prev is SendBuffered<*>) {
5253
if (!prev.remove()) {
5354
prev.helpRemove()
5455
}
55-
prev = prev.prevNode
56+
prev = prev.prevNodeIfNotRemoved ?: return
5657
}
5758
}
5859

kotlinx-coroutines-core/common/src/internal/LockFreeLinkedList.common.kt

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ public expect open class LockFreeLinkedListNode() {
1111
public val isRemoved: Boolean
1212
public val nextNode: LockFreeLinkedListNode
1313
public val prevNode: LockFreeLinkedListNode
14+
public val prevNodeIfNotRemoved: LockFreeLinkedListNode?
1415
public fun addLast(node: LockFreeLinkedListNode)
1516
public fun addOneIfEmpty(node: LockFreeLinkedListNode): Boolean
1617
public inline fun addLastIf(node: LockFreeLinkedListNode, crossinline condition: () -> Boolean): Boolean

kotlinx-coroutines-core/js/src/internal/LinkedList.kt

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public open class LinkedListNode {
2323

2424
public inline val nextNode get() = _next
2525
public inline val prevNode get() = _prev
26+
public inline val prevNodeIfNotRemoved: LockFreeLinkedListNode? get() = _prev.takeIf { !_removed }
2627
public inline val isRemoved get() = _removed
2728

2829
public fun addLast(node: Node) {

kotlinx-coroutines-core/jvm/src/internal/LockFreeLinkedList.kt

+11-15
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ public actual open class LockFreeLinkedListNode {
108108
public actual val prevNode: Node
109109
get() = correctPrev(null) ?: findPrevNonRemoved(_prev.value)
110110

111+
public actual val prevNodeIfNotRemoved: LockFreeLinkedListNode?
112+
get() = correctPrev(null)
113+
111114
private tailrec fun findPrevNonRemoved(current: Node): Node {
112115
if (!current.isRemoved) return current
113116
return findPrevNonRemoved(current._prev.value)
@@ -258,21 +261,13 @@ public actual open class LockFreeLinkedListNode {
258261
// Helps with removal of this node
259262
public actual fun helpRemove() {
260263
// Note: this node must be already removed
261-
(next as Removed).ref.helpRemovePrev()
264+
(next as Removed).ref.correctPrev(null)
262265
}
263266

264267
// Helps with removal of nodes that are previous to this
265-
private fun helpRemovePrev() {
266-
// We need to call correctPrev on a non-removed node to ensure progress, since correctPrev bails out when
267-
// called on a removed node. There's always at least one non-removed node (list head).
268-
var node = this
269-
while (true) {
270-
val next = node.next
271-
if (next !is Removed) break
272-
node = next.ref
273-
}
274-
// Found a non-removed node
275-
node.correctPrev(null)
268+
@PublishedApi
269+
internal fun helpRemovePrev() {
270+
correctPrev(null)
276271
}
277272

278273
public actual fun removeFirstOrNull(): Node? {
@@ -309,13 +304,14 @@ public actual open class LockFreeLinkedListNode {
309304
* However, we limit the number of attempts to move to the next node and help with removal at the end
310305
* to avoid repeatedly scanning very long lists in LinkedListChannel.
311306
*/
312-
if (moveNextAttempts++ < 32) {
307+
if (moveNextAttempts++ < 4) {
313308
first = next
314309
} else {
315-
break // help and start from the beginning
310+
// help and start from the beginning
311+
next.helpRemovePrev()
312+
break
316313
}
317314
}
318-
first.helpRemove() // help remove this one and retry from the beginning of the list
319315
}
320316
}
321317

kotlinx-coroutines-core/native/src/internal/LinkedList.kt

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public open class LinkedListNode {
2121

2222
public inline val nextNode get() = _next
2323
public inline val prevNode get() = _prev
24+
public inline val prevNodeIfNotRemoved: LockFreeLinkedListNode? get() = _prev.takeIf { !_removed }
2425
public inline val isRemoved get() = _removed
2526

2627
public fun addLast(node: Node) {

0 commit comments

Comments
 (0)