Fixed
Status Update
No update yet.
Comments
cs...@google.com
FATAL EXCEPTION: DefaultDispatcher-worker-9
Process: work, PID: 7773
java.util.ConcurrentModificationException
at
androidx.compose.runtime.snapshots.StateListIterator.validateModification(SnapshotStateList.kt:297)
at androidx.compose.runtime.snapshots.StateListIterator.next(SnapshotStateList.kt:276)
at
.DashboardViewModel$getCarousalListApi$1$1$1.invokeSuspend(DashboardViewModel.kt:751)
at
.DashboardViewModel$getCarousalListApi$1$1$1.invoke(Unknown
Source:8)
at
.DashboardViewModel$getCarousalListApi$1$1$1.invoke(Unknown
Source:2)
at
.config.common.ExtensionsKt$launchOnIOWithCompletion$1.invokeSuspend(Extensions.kt:28)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@5443283, Dispatchers.IO]
Process: work, PID: 7773
java.util.ConcurrentModificationException
at
androidx.compose.runtime.snapshots.StateListIterator.validateModification(SnapshotStateList.kt:297)
at androidx.compose.runtime.snapshots.StateListIterator.next(SnapshotStateList.kt:276)
at
.DashboardViewModel$getCarousalListApi$1$1$1.invokeSuspend(DashboardViewModel.kt:751)
at
.DashboardViewModel$getCarousalListApi$1$1$1.invoke(Unknown
Source:8)
at
.DashboardViewModel$getCarousalListApi$1$1$1.invoke(Unknown
Source:2)
at
.config.common.ExtensionsKt$launchOnIOWithCompletion$1.invokeSuspend(Extensions.kt:28)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@5443283, Dispatchers.IO]
ap...@google.com
reverse() constructs two iterators (if the list is > 18 in size), one forward walking and one backwards walking and will modify the array using both iterators. This is currently treated as a concurrent modification as the array was modified after the iterator was created. This is too strict for the Java algorithms. They expect a concurrent modification exception to only be thrown if a structural chagne to the list is made, e.g. an insert or delete, not a value change.
First, mutableStateListOf() lists will be changed to not interpret setting the value of the slot of an element as a concurrent. Second, they will be changed to support RandomAccess which avoids the algorithms using the iterators for operations that would be faster if they used get and set directly.
ch...@google.com
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit 75f720e1695c3574930d0385211341fac876a834
Author: Chuck Jazdzewski <chuckj@google.com>
Date: Thu Sep 14 15:22:33 2023
Relax concurrent modification exception for snapshot state list
The snapshot state list iterator considered any change made outside the
iterator as a concurrent change. This change relaxes the checking to
only consider structural changes (adding and removing) to be treated as
a concurrent change. This allows SnapshotStateList to be used with
`reverse()` which assumes that setting the value of an element is not a
concurrent change.
Adds RandomeAccess interface to MutableStateList to signal collection
extensions functions they can use direct indexing versions of their
algorithm.
Corrects the set() after a call previous() to update the correct
element.
Fixes: 219554654
Test: ./gradlew :compose:r:r:tDUT
Relnote: """SnapshotStateList is not marked as RandomAccess to enable
the direct indexing version of list helpers to be used"
Change-Id: I5210ca5c0f490619381ecf93ac0b1ccb03071e36
M compose/runtime/runtime/api/current.txt
M compose/runtime/runtime/api/restricted_current.txt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/SnapshotStateList.kt
M compose/runtime/runtime/src/nonEmulatorCommonTest/kotlin/androidx/compose/runtime/snapshots/SnapshotStateListTests.kt
https://android-review.googlesource.com/2751617
Branch: androidx-main
commit 75f720e1695c3574930d0385211341fac876a834
Author: Chuck Jazdzewski <chuckj@google.com>
Date: Thu Sep 14 15:22:33 2023
Relax concurrent modification exception for snapshot state list
The snapshot state list iterator considered any change made outside the
iterator as a concurrent change. This change relaxes the checking to
only consider structural changes (adding and removing) to be treated as
a concurrent change. This allows SnapshotStateList to be used with
`reverse()` which assumes that setting the value of an element is not a
concurrent change.
Adds RandomeAccess interface to MutableStateList to signal collection
extensions functions they can use direct indexing versions of their
algorithm.
Corrects the set() after a call previous() to update the correct
element.
Fixes: 219554654
Test: ./gradlew :compose:r:r:tDUT
Relnote: """SnapshotStateList is not marked as RandomAccess to enable
the direct indexing version of list helpers to be used"
Change-Id: I5210ca5c0f490619381ecf93ac0b1ccb03071e36
M compose/runtime/runtime/api/current.txt
M compose/runtime/runtime/api/restricted_current.txt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/snapshots/SnapshotStateList.kt
M compose/runtime/runtime/src/nonEmulatorCommonTest/kotlin/androidx/compose/runtime/snapshots/SnapshotStateListTests.kt
na...@google.com
ae...@google.com
I temporarily removed the first performance optimization aosp/2636327 in aosp/2676415 because it caused a bug with subcomposition
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit 3118e88c6485cb8003b30e6bc09ee302ca4557ce
Author: Chuck Jazdzewski <chuckj@google.com>
Date: Thu Jul 27 16:30:42 2023
Fix and re-enable providers optimization
Disabled the single provider optimization because it could cause
the composition local scope to be incorrect.
It was incorrect when the provider cache was invalidated by a
provider closing just before the composer scope was read again
which causes it to re-lookup the current provider scope. The
`startProvider` call neglected to set the `writerHasAProvider` that
would cause the writer to be inspected for a provider not just the
reader (as the provider is being added during the composition).
The fix is to add this flag at the same location it is in
`startProviders()`.
Bug: 292224893
Fixes: 288169379
Test: ./gradlew :compose:r:r:tDUT
Change-Id: I883c3ba691e41821944a3020eaa1089130cf11a3
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composer.kt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/CompositionLocal.kt
M compose/runtime/runtime/src/nonEmulatorCommonTest/kotlin/androidx/compose/runtime/CompositionLocalTests.kt
https://android-review.googlesource.com/2680337
Branch: androidx-main
commit 3118e88c6485cb8003b30e6bc09ee302ca4557ce
Author: Chuck Jazdzewski <chuckj@google.com>
Date: Thu Jul 27 16:30:42 2023
Fix and re-enable providers optimization
Disabled the single provider optimization because it could cause
the composition local scope to be incorrect.
It was incorrect when the provider cache was invalidated by a
provider closing just before the composer scope was read again
which causes it to re-lookup the current provider scope. The
`startProvider` call neglected to set the `writerHasAProvider` that
would cause the writer to be inspected for a provider not just the
reader (as the provider is being added during the composition).
The fix is to add this flag at the same location it is in
`startProviders()`.
Bug: 292224893
Fixes: 288169379
Test: ./gradlew :compose:r:r:tDUT
Change-Id: I883c3ba691e41821944a3020eaa1089130cf11a3
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composer.kt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/CompositionLocal.kt
M compose/runtime/runtime/src/nonEmulatorCommonTest/kotlin/androidx/compose/runtime/CompositionLocalTests.kt
na...@google.com
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.compose.runtime:runtime:1.6.0-alpha03androidx.compose.runtime:runtime-android:1.6.0-alpha03androidx.core:core-testing:1.12.0-rc01
Description
Summary
The current API for providing composition locals is very allocation heavy and requires several groups. This can be improved in several ways,
The current API will allocated an array to store the providers and a mall object to for each "provides" parameter. It will then create a persistent map of the provided values and then union of the existing map and this map as the persistent scope.
Some composable function just provide one provides which makes these allocation particularly heavey weight. Also, constructing the union persisent map can be expensive.
Proposals
1. Special case single provider
The single provider can be special cased to avoid the creating of the array, the
ProvidesValueinstance and the persistent map.This would require new API that takes the composition local definition and the value (as well as a corresponding lint with a quick-fix to migrate code to the new API).
2. Allow building the map directly outside composition.
Instead of building the map in composition, the local map (for large maps) can be constructed as helper or outside composition entirely. This is specifically helpful when the map is conditionally built the current API requires paying the local provider overhead multiple times for shared or conditional providers.
This will require a new API that allows providing a composition local map directly.
3. Faster persistent maps
The persistent map is a general map that handles many cases well but it doesn't handle small changes well (such as adding a single value). We could provide a wrapper around the persistent map that will handles the special cases found in the Compose library better such as a singleton wrapper for cases, such when a single value is provided.
This doesn't require any API changes.