From 95892c273d6a2a52c0ee664867a71c534e41ac07 Mon Sep 17 00:00:00 2001 From: Pablo Baxter Date: Thu, 1 Sep 2022 21:33:38 -0700 Subject: [PATCH 1/4] Ensure `awaitFrame()` only awaits a single frame when used from the main thread --- ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt | 9 ++++++--- .../test/HandlerDispatcherTest.kt | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt b/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt index 5e33169dd1..91cd2abe98 100644 --- a/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt +++ b/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt @@ -190,11 +190,14 @@ public suspend fun awaitFrame(): Long { postFrameCallback(choreographer, cont) } } - // post into looper thread to figure it out return suspendCancellableCoroutine { cont -> - Dispatchers.Main.dispatch(EmptyCoroutineContext, Runnable { + if (Looper.myLooper() === Looper.getMainLooper()) { // Check if we are already in the main looper thread updateChoreographerAndPostFrameCallback(cont) - }) + } else { // post into looper thread to figure it out + Dispatchers.Main.dispatch(EmptyCoroutineContext, Runnable { + updateChoreographerAndPostFrameCallback(cont) + }) + } } } diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt index fe97ae8d27..c016f35533 100644 --- a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt @@ -109,14 +109,14 @@ class HandlerDispatcherTest : TestBase() { launch(Dispatchers.Main, start = CoroutineStart.UNDISPATCHED) { expect(1) awaitFrame() - expect(5) + expect(3) } expect(2) // Run choreographer detection mainLooper.runOneTask() - expect(3) - mainLooper.scheduler.advanceBy(50, TimeUnit.MILLISECONDS) expect(4) + mainLooper.scheduler.advanceBy(50, TimeUnit.MILLISECONDS) + expect(5) mainLooper.scheduler.advanceBy(51, TimeUnit.MILLISECONDS) finish(6) } From 0768ee5f5b7ea662420b52c9428a0a2ec7a462fb Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Tue, 6 Sep 2022 11:41:22 +0200 Subject: [PATCH 2/4] ~cleanup --- .../src/HandlerDispatcher.kt | 24 ++++++------ ui/kotlinx-coroutines-javafx/build.gradle.kts | 37 ------------------- .../src/JavaFxDispatcher.kt | 11 +++--- .../src/SwingDispatcher.kt | 17 +++------ 4 files changed, 25 insertions(+), 64 deletions(-) diff --git a/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt b/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt index 91cd2abe98..7012c23ecd 100644 --- a/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt +++ b/ui/kotlinx-coroutines-android/src/HandlerDispatcher.kt @@ -185,25 +185,27 @@ private var choreographer: Choreographer? = null public suspend fun awaitFrame(): Long { // fast path when choreographer is already known val choreographer = choreographer - if (choreographer != null) { - return suspendCancellableCoroutine { cont -> + return if (choreographer != null) { + suspendCancellableCoroutine { cont -> postFrameCallback(choreographer, cont) } + } else { + awaitFrameSlowPath() } - return suspendCancellableCoroutine { cont -> - if (Looper.myLooper() === Looper.getMainLooper()) { // Check if we are already in the main looper thread +} + +private suspend fun awaitFrameSlowPath(): Long = suspendCancellableCoroutine { cont -> + if (Looper.myLooper() === Looper.getMainLooper()) { // Check if we are already in the main looper thread + updateChoreographerAndPostFrameCallback(cont) + } else { // post into looper thread to figure it out + Dispatchers.Main.dispatch(cont.context, Runnable { updateChoreographerAndPostFrameCallback(cont) - } else { // post into looper thread to figure it out - Dispatchers.Main.dispatch(EmptyCoroutineContext, Runnable { - updateChoreographerAndPostFrameCallback(cont) - }) - } + }) } } private fun updateChoreographerAndPostFrameCallback(cont: CancellableContinuation) { - val choreographer = choreographer ?: - Choreographer.getInstance()!!.also { choreographer = it } + val choreographer = choreographer ?: Choreographer.getInstance()!!.also { choreographer = it } postFrameCallback(choreographer, cont) } diff --git a/ui/kotlinx-coroutines-javafx/build.gradle.kts b/ui/kotlinx-coroutines-javafx/build.gradle.kts index f9f66249eb..456ced137c 100644 --- a/ui/kotlinx-coroutines-javafx/build.gradle.kts +++ b/ui/kotlinx-coroutines-javafx/build.gradle.kts @@ -21,40 +21,3 @@ javafx { modules = listOf("javafx.controls") configuration = "javafx" } - -val JDK_18: String? by lazy { - System.getenv("JDK_18") -} - -val checkJdk8 by tasks.registering { - // only fail w/o JDK_18 when actually trying to test, not during project setup phase - doLast { - if (JDK_18 == null) { - throw GradleException( - """ - JDK_18 environment variable is not defined. - Can't run JDK 8 compatibility tests. - Please ensure JDK 8 is installed and that JDK_18 points to it. - """.trimIndent() - ) - } - } -} - -val jdk8Test by tasks.registering(Test::class) { - // Run these tests only during nightly stress test - onlyIf { project.properties["stressTest"] != null } - - val test = tasks.test.get() - - classpath = test.classpath - testClassesDirs = test.testClassesDirs - executable = "$JDK_18/bin/java" - - dependsOn("compileTestKotlin") - dependsOn(checkJdk8) -} - -tasks.build { - dependsOn(jdk8Test) -} diff --git a/ui/kotlinx-coroutines-javafx/src/JavaFxDispatcher.kt b/ui/kotlinx-coroutines-javafx/src/JavaFxDispatcher.kt index d158fb745a..61583aad65 100644 --- a/ui/kotlinx-coroutines-javafx/src/JavaFxDispatcher.kt +++ b/ui/kotlinx-coroutines-javafx/src/JavaFxDispatcher.kt @@ -34,7 +34,7 @@ public sealed class JavaFxDispatcher : MainCoroutineDispatcher(), Delay { /** @suppress */ override fun scheduleResumeAfterDelay(timeMillis: Long, continuation: CancellableContinuation) { - val timeline = schedule(timeMillis, TimeUnit.MILLISECONDS) { + val timeline = schedule(timeMillis) { with(continuation) { resumeUndispatched(Unit) } } continuation.invokeOnCancellation { timeline.stop() } @@ -42,14 +42,14 @@ public sealed class JavaFxDispatcher : MainCoroutineDispatcher(), Delay { /** @suppress */ override fun invokeOnTimeout(timeMillis: Long, block: Runnable, context: CoroutineContext): DisposableHandle { - val timeline = schedule(timeMillis, TimeUnit.MILLISECONDS) { + val timeline = schedule(timeMillis) { block.run() } return DisposableHandle { timeline.stop() } } - private fun schedule(time: Long, unit: TimeUnit, handler: EventHandler): Timeline = - Timeline(KeyFrame(Duration.millis(unit.toMillis(time).toDouble()), handler)).apply { play() } + private fun schedule(timeMillis: Long, handler: EventHandler): Timeline = + Timeline(KeyFrame(Duration.millis(timeMillis.toDouble()), handler)).apply { play() } } internal class JavaFxDispatcherFactory : MainDispatcherFactory { @@ -97,7 +97,7 @@ public suspend fun awaitPulse(): Long = suspendCancellableCoroutine { cont -> } private class PulseTimer : AnimationTimer() { - val next = CopyOnWriteArrayList>() + private val next = CopyOnWriteArrayList>() override fun handle(now: Long) { val cur = next.toTypedArray() @@ -116,6 +116,7 @@ internal fun initPlatform(): Boolean = PlatformInitializer.success // Lazily try to initialize JavaFx platform just once private object PlatformInitializer { + @JvmField val success = run { /* * Try to instantiate JavaFx platform in a way which works diff --git a/ui/kotlinx-coroutines-swing/src/SwingDispatcher.kt b/ui/kotlinx-coroutines-swing/src/SwingDispatcher.kt index 3b43483dbc..010f18c6c2 100644 --- a/ui/kotlinx-coroutines-swing/src/SwingDispatcher.kt +++ b/ui/kotlinx-coroutines-swing/src/SwingDispatcher.kt @@ -7,7 +7,6 @@ package kotlinx.coroutines.swing import kotlinx.coroutines.* import kotlinx.coroutines.internal.* import java.awt.event.* -import java.util.concurrent.* import javax.swing.* import kotlin.coroutines.* @@ -29,26 +28,22 @@ public sealed class SwingDispatcher : MainCoroutineDispatcher(), Delay { /** @suppress */ override fun scheduleResumeAfterDelay(timeMillis: Long, continuation: CancellableContinuation) { - val timer = schedule(timeMillis, TimeUnit.MILLISECONDS, ActionListener { + val timer = schedule(timeMillis) { with(continuation) { resumeUndispatched(Unit) } - }) + } continuation.invokeOnCancellation { timer.stop() } } /** @suppress */ override fun invokeOnTimeout(timeMillis: Long, block: Runnable, context: CoroutineContext): DisposableHandle { - val timer = schedule(timeMillis, TimeUnit.MILLISECONDS, ActionListener { + val timer = schedule(timeMillis) { block.run() - }) - return object : DisposableHandle { - override fun dispose() { - timer.stop() - } } + return DisposableHandle { timer.stop() } } - private fun schedule(time: Long, unit: TimeUnit, action: ActionListener): Timer = - Timer(unit.toMillis(time).coerceAtMost(Int.MAX_VALUE.toLong()).toInt(), action).apply { + private fun schedule(timeMillis: Long, action: ActionListener): Timer = + Timer(timeMillis.coerceAtMost(Int.MAX_VALUE.toLong()).toInt(), action).apply { isRepeats = false start() } From dcd2c0497d83069fc6dc14b0bb5bb7189fda78a9 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 15 Sep 2022 17:42:00 +0200 Subject: [PATCH 3/4] ~cleanup test --- ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt index c016f35533..0d05edeef8 100644 --- a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt @@ -116,9 +116,7 @@ class HandlerDispatcherTest : TestBase() { mainLooper.runOneTask() expect(4) mainLooper.scheduler.advanceBy(50, TimeUnit.MILLISECONDS) - expect(5) - mainLooper.scheduler.advanceBy(51, TimeUnit.MILLISECONDS) - finish(6) + finish(5) } private fun CoroutineScope.doTestAwaitWithDetectedChoreographer() { From 51436d6b5d997d8cfa7d3e0fa9890876133017bc Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Fri, 16 Sep 2022 15:34:18 +0200 Subject: [PATCH 4/4] ~ --- ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt index 0d05edeef8..afe6cff2f6 100644 --- a/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt +++ b/ui/kotlinx-coroutines-android/test/HandlerDispatcherTest.kt @@ -114,9 +114,7 @@ class HandlerDispatcherTest : TestBase() { expect(2) // Run choreographer detection mainLooper.runOneTask() - expect(4) - mainLooper.scheduler.advanceBy(50, TimeUnit.MILLISECONDS) - finish(5) + finish(4) } private fun CoroutineScope.doTestAwaitWithDetectedChoreographer() {