Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a regression in JobSupport memory consumption #4110

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion kotlinx-coroutines-core/common/src/Await.kt
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,6 @@ private class AwaitAll<T>(private val deferreds: Array<out Deferred<T>>) {
}
}

override val onCancelling = false
override val onCancelling get() = false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -675,5 +675,5 @@ private class ChildContinuation(
child.parentCancelled(child.getContinuationCancellationCause(job))
}

override val onCancelling = true
override val onCancelling get() = true
}
2 changes: 1 addition & 1 deletion kotlinx-coroutines-core/common/src/Job.kt
Original file line number Diff line number Diff line change
Expand Up @@ -673,5 +673,5 @@ private class DisposeOnCompletion(
) : JobNode() {
override fun invoke(cause: Throwable?) = handle.dispose()

override val onCancelling = false
override val onCancelling get() = false
}
12 changes: 6 additions & 6 deletions kotlinx-coroutines-core/common/src/JobSupport.kt
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ public open class JobSupport constructor(active: Boolean) : Job, ChildJob, Paren
override fun invoke(cause: Throwable?) {
select.trySelect(this@JobSupport, Unit)
}
override val onCancelling: Boolean = false
override val onCancelling: Boolean get() = false
}

/**
Expand Down Expand Up @@ -1410,14 +1410,14 @@ private class InvokeOnCompletion(
private val handler: CompletionHandler
) : JobNode() {
override fun invoke(cause: Throwable?) = handler.invoke(cause)
override val onCancelling = false
override val onCancelling get() = false
}

private class ResumeOnCompletion(
private val continuation: Continuation<Unit>
) : JobNode() {
override fun invoke(cause: Throwable?) = continuation.resume(Unit)
override val onCancelling = false
override val onCancelling get() = false
}

private class ResumeAwaitOnCompletion<T>(
Expand All @@ -1435,7 +1435,7 @@ private class ResumeAwaitOnCompletion<T>(
continuation.resume(state.unboxState() as T)
}
}
override val onCancelling = false
override val onCancelling get() = false
}

// -------- invokeOnCancellation nodes
Expand All @@ -1448,7 +1448,7 @@ private class InvokeOnCancelling(
override fun invoke(cause: Throwable?) {
if (_invoked.compareAndSet(expect = false, update = true)) handler.invoke(cause)
}
override val onCancelling = true
override val onCancelling get() = true
}

private class ChildHandleNode(
Expand All @@ -1457,5 +1457,5 @@ private class ChildHandleNode(
override val parent: Job get() = job
override fun invoke(cause: Throwable?) = childJob.parentCancelled(job)
override fun childCancelled(cause: Throwable): Boolean = job.childCancelled(cause)
override val onCancelling: Boolean = true
override val onCancelling: Boolean get() = true
}
2 changes: 1 addition & 1 deletion kotlinx-coroutines-core/jvm/src/Future.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private class CancelFutureOnCompletion(
if (cause != null) future.cancel(false)
}

override val onCancelling = false
override val onCancelling get() = false
}

private class CancelFutureOnCancel(private val future: Future<*>) : CancelHandler {
Expand Down
2 changes: 1 addition & 1 deletion kotlinx-coroutines-core/jvm/src/Interruptible.kt
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private class ThreadState : JobNode() {
}
}

override val onCancelling = true
override val onCancelling get() = true

private fun invalidState(state: Int): Nothing = error("Illegal state $state")
}
23 changes: 22 additions & 1 deletion kotlinx-coroutines-core/jvm/test/MemoryFootprintTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package kotlinx.coroutines

import kotlinx.coroutines.testing.*
import org.junit.Test
import org.openjdk.jol.info.ClassLayout
import org.openjdk.jol.info.*
import kotlin.test.*


Expand All @@ -11,6 +11,22 @@ class MemoryFootprintTest : TestBase(true) {
@Test
fun testJobLayout() = assertLayout(Job().javaClass, 24)

@Test
fun testJobSize() {
assertTotalSize(jobWithChildren(1), 112)
assertTotalSize(jobWithChildren(2), 192) // + 80
assertTotalSize(jobWithChildren(3), 248) // + 56
assertTotalSize(jobWithChildren(4), 304) // + 56
}

private fun jobWithChildren(numberOfChildren: Int): Job {
val result = Job()
repeat(numberOfChildren) {
Job(result)
}
return result
}

@Test
fun testCancellableContinuationFootprint() = assertLayout(CancellableContinuationImpl::class.java, 48)

Expand All @@ -19,4 +35,9 @@ class MemoryFootprintTest : TestBase(true) {
// println(ClassLayout.parseClass(clz).toPrintable())
assertEquals(expectedSize.toLong(), size)
}

private fun assertTotalSize(instance: Job, expectedSize: Int) {
val size = GraphLayout.parseInstance(instance).totalSize()
assertEquals(expectedSize.toLong(), size)
}
}