Final recomposition does not run in simple setup [178904648]

Obsolete

Bug

Status Update

No update yet.

Description

go...@jakewharton.com

created issue #1

Jan 31, 2021 03:09AM

I'm using Compose at git SHA 53d68b9c402a4abffaaa9ac34a7f9ac91e4b0a0b which is the latest as of Friday night so that I can try out http://r.android.com/1562374.

Given the code:

fun main() = runBlocking {
  var hasFrameWaiters = true
  val clock = BroadcastFrameClock {
    println("Waiting for frame")
    hasFrameWaiters = true
  }

  val job = Job(coroutineContext[Job])
  val composeContext = coroutineContext + clock + job

  val recomposer = Recomposer(composeContext)
  val composition = Composition(UnitApplier, recomposer)

  launch(start = CoroutineStart.UNDISPATCHED, context = composeContext) {
    println("Recomposing!")
    recomposer.runRecomposeAndApplyChanges()
  }

  launch(context = composeContext) {
    while (true) {
      println("Clock tick!")
      clock.sendFrame(0L) // Frame time value is not used by Compose runtime.

      if (hasFrameWaiters) {
        hasFrameWaiters = false

        println("Frame displayed!")
      }

      delay(50)
    }
  }

  var snapshotNotificationsPending = false
  val observer: (state: Any) -> Unit = {
    if (!snapshotNotificationsPending) {
      snapshotNotificationsPending = true
      println("Scheduling apply notifications")
      launch {
        println("Sending apply notifications")
        snapshotNotificationsPending = false
        Snapshot.sendApplyNotifications()
      }
    }
  }
  Snapshot.registerGlobalWriteObserver(observer)

  composition.setContent {
    var count by remember { mutableStateOf(0) }

    println("Composition run $count")

    LaunchedEffect(Unit) {
      for (i in 1..20) {
        delay(250)
        println("Assign $i")
        count = i
      }
    }
  }

  recomposer.close()
  recomposer.join()

  println("End")
  job.cancel()
  composition.dispose()
}

object UnitApplier : AbstractApplier<Unit>(Unit) {
  override fun insertTopDown(index: Int, instance: Unit) {}
  override fun insertBottomUp(index: Int, instance: Unit) {}
  override fun remove(index: Int, count: Int) {}
  override fun move(from: Int, to: Int, count: Int) {}
  override fun onClear() {}
}

This ends with:

Assign 19
Scheduling apply notifications
Sending apply notifications
Waiting for frame
Clock tick!
Composition run 19
Frame displayed!
Clock tick!
Clock tick!
Clock tick!
Clock tick!
Assign 20
Scheduling apply notifications
Sending apply notifications
End

You'll notice that on the final assignment of 20 (which allows the effect job to complete fully), there is no one marked as waiting for the frame. This should have happened inside runRecomposeAndApplyChanges()'s while loop.

If you change the sample to suspend fun main() = coroutineScope {, the various launch's run on different threads which does produce the correct output. However, I want this all to run on the main thread via runBlocking.

Comments

go...@jakewharton.com <go...@jakewharton.com> #2Jan 31, 2021 03:24AM

Project: platform/frameworks/support
Branch: androidx-main
Author: Kuan-Ying Chou <kuanyingchou@google.com>
Link: https://android-review.googlesource.com/3328036

Remove ToT dependencies in wear:compose:compose-navigation

Expand for full commit details

Remove ToT dependencies in wear:compose:compose-navigation 
 
Bug: 374102924 
Test: n/a 
Change-Id: Id057c7254a0282eab77a2facb4d278e3b5ccf784

Files:

M wear/compose/compose-navigation/samples/build.gradle

Hash: 285f596b323a37b82e07621acb990847b8cf4328
Date: Thu Oct 31 13:36:53 2024

cl...@google.com <cl...@google.com> Feb 1, 2021 01:55PM

Assigned to ad...@google.com.

go...@jakewharton.com <go...@jakewharton.com> #3Feb 27, 2021 03:42AM

Project: platform/frameworks/support
Branch: androidx-main
Author: Kuan-Ying Chou <kuanyingchou@google.com>
Link: https://android-review.googlesource.com/3323381

Update Kotlin targets to 1.9

Expand for full commit details

Update Kotlin targets to 1.9 
 
Update Kotlin target of savedstate and projects that depend on it to 1.9. 
 
Bug: 374102924 
Test: existing tests still pass 
Change-Id: I442310ec57f7db381c16f95985a952deb9af57b2

Files:

M lifecycle/lifecycle-viewmodel-compose/build.gradle
M lifecycle/lifecycle-viewmodel-compose/samples/build.gradle
M lifecycle/lifecycle-viewmodel-savedstate/build.gradle
M lifecycle/lifecycle-viewmodel-testing/build.gradle
M savedstate/savedstate-ktx/build.gradle
M savedstate/savedstate/build.gradle
M savedstate/savedstate/src/nativeMain/kotlin/androidx/savedstate/internal/SynchronizedObject.native.kt

Hash: 73305d3cf9d9bfff42b1836ac0127032b7721496
Date: Tue Oct 29 17:27:51 2024

go...@jakewharton.com <go...@jakewharton.com> #4Apr 1, 2021 03:50AM

Project: platform/frameworks/support
Branch: androidx-main
Author: Kuan-Ying Chou <kuanyingchou@google.com>
Link: https://android-review.googlesource.com/3313396

Add functions to encode serializable objects to and decode serializable objects from SavedState.

Expand for full commit details

Add functions to encode serializable objects to and decode serializable objects from `SavedState`. 
 
We do this by implementing an `Encoder` and a `Decoder` that writes and reads a `SavedState` output format for Kotlin Serialization (https://github.com/Kotlin/kotlinx.serialization). The format represents a Kotlin Serialization "primitive" as their corresponding `SavedState` supported type, and represents a "composite" in a `SavedState` with its property names as keys. Nested composites are represented by nested `SavedState`s. 
 
Here are some other design choices we made: 
 
1. We don't record size for collections and rely on the size of the `SavedState` in decoding so that collections and non-collections are treated the same. 
2. To save space, we don't encode default values by default (it can be tweaked with `@EncodeDefault` with plugin-generated serializers). 
3. To support nullable parameters with default arguments (e.g. `val a: String? = "foo"`) and for `SavedState` size to be correct for collections, we encode `null`s. 
4. To keep things simple we don't support specifying custom `SerializersModule`s for now. 
 
Please note that when using Kotlin Serialization and these functions as a `Parcelable` alternative on Android there's a performance overhead because of the extra serialization needed. 
 
Also note that on Android we don't have built-in support for Android or Java specific types supported by `Bundle` or `Parcel` yet (e.g. `Exception`, `java.io.Serializable`, or `IBinder`).  We may consider adding these types in the future. 
 
Relnote: Add encodeToSavedState() and decodeFromSavedState() functions 
Test: SavedStateCodecTest.kt 
Bug: 374102924 
Change-Id: I6f59faffaa3777bf56132a67f41b867d7a9663e5

Files:

M development/build_log_simplifier/messages.ignore
M docs-tip-of-tree/build.gradle
A savedstate/savedstate-samples/build.gradle
A savedstate/savedstate-samples/src/main/java/androidx/savedstate/SavedStateCodecSamples.kt
M savedstate/savedstate/api/current.txt
M savedstate/savedstate/api/restricted_current.txt
M savedstate/savedstate/bcv/native/current.txt
M savedstate/savedstate/build.gradle
A savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateCodecAndroidTest.android.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/serialization/SavedStateDecoder.kt
A savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/serialization/SavedStateEncoder.kt
A savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateCodecTest.kt
A savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateCodecTestUtils.kt
M settings.gradle

Hash: 57162ea74b646ef7ae43397251fd4cd32e2d80c3
Date: Thu Oct 17 19:10:05 2024

go...@jakewharton.com <go...@jakewharton.com> #5Apr 1, 2021 04:04AM

Good news! Wear Compose has been updated to ab/12605136 in

https://critique.corp.google.com/cl/693389210 and this bug has been affected.

go...@jakewharton.com <go...@jakewharton.com> #6Apr 1, 2021 04:14AM

Hacked with 😬

diff --git compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt
index 3da5ece0b6e..ebdd9022851 100644
--- compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt
+++ compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt
@@ -393,18 +393,30 @@ class Recomposer(
     suspend fun runRecomposeAndApplyChanges() = recompositionRunner { parentFrameClock ->
         val toRecompose = mutableListOf<ControlledComposition>()
         val toApply = mutableListOf<ControlledComposition>()
-        while (shouldKeepRecomposing) {
-            awaitWorkAvailable()

-            // Don't await a new frame if we don't have frame-scoped work
-            if (
-                synchronized(stateLock) {
-                    if (!hasFrameWorkLocked) {
-                        recordComposerModificationsLocked()
-                        !hasFrameWorkLocked
-                    } else false
+        // Somewhat yikes workaround for https://issuetracker.google.com/issues/178904648.
+        var lastFrameHack = false
+        while (true) {
+            if (!shouldKeepRecomposing) {
+                // Assume that an effect job mutated state as its last action and requires that we
+                // recompose one more time.
+                if (lastFrameHack) {
+                    break
                 }
-            ) continue
+                lastFrameHack = true
+            } else {
+                awaitWorkAvailable()
+
+                // Don't await a new frame if we don't have frame-scoped work
+                if (
+                    synchronized(stateLock) {
+                        if (!hasFrameWorkLocked) {
+                            recordComposerModificationsLocked()
+                            !hasFrameWorkLocked
+                        } else false
+                    }
+                ) continue
+            }

             // Align work with the next frame to coalesce changes.
             // Note: it is possible to resume from the above with no recompositions pending,

go...@jakewharton.com <go...@jakewharton.com> #7Apr 1, 2021 04:28AM

Even that seems insufficient in practice...

al...@google.com <al...@google.com> #8Mar 14, 2024 08:23PM

Status: Won't Fix (Obsolete)

Closing all P3/4 issues older than 2023-01-01.

If this issue is still relevant, please file a new report. Do not comment on this issue.

Issue 178904648

Description

Issue summary

Comments

go...@jakewharton.com <go...@jakewharton.com> #2Jan 31, 2021 03:24AM

cl...@google.com <cl...@google.com> Feb 1, 2021 01:55PM

go...@jakewharton.com <go...@jakewharton.com> #3Feb 27, 2021 03:42AM

go...@jakewharton.com <go...@jakewharton.com> #4Apr 1, 2021 03:50AM

go...@jakewharton.com <go...@jakewharton.com> #5Apr 1, 2021 04:04AM

go...@jakewharton.com <go...@jakewharton.com> #6Apr 1, 2021 04:14AM

go...@jakewharton.com <go...@jakewharton.com> #7Apr 1, 2021 04:28AM

al...@google.com <al...@google.com> #8Mar 14, 2024 08:23PM

Add comment

Issue metadata