Status Update
Comments
au...@google.com <au...@google.com> #2
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
al...@google.com <al...@google.com> #3
co...@gmail.com <co...@gmail.com> #4
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
co...@gmail.com <co...@gmail.com> #5
Actually our biggest offender (BottomNavigationView) does not have any Paging influence in any of the other threads, so I think it's not solely to blame.
al...@google.com <al...@google.com> #6
There's a DEBUG
field in SimpleArrayMap
that could be helpful, but honestly my recommendation would be to avoid SimpleArrayMap
unless you can fully validate the safety of your own usages as well as your libraries' usages.
We see a ton of issues from this class in Google apps, as well, and I don't think the performance trade-off is worth the risk.
Just in case Paging is somehow involved, I'm reassigning this issue for further investigation.
al...@google.com <al...@google.com> #8
Note that the stack trace for the failure isn't meaningful in this case. The cache could have been corrupted by any access to the class within your app process.
co...@gmail.com <co...@gmail.com> #9
There's a DEBUG field in SimpleArrayMap that could be helpful, but honestly my recommendation would be to avoid SimpleArrayMap unless you can fully validate the safety of your own usages
We actually don't use it at all, the issue seems to be coming from View classes within the Framework that use it.
al...@google.com <al...@google.com> #10
Oh. Oh no.
So, for what it's worth, if you don't have any usages in your app then it's down to libraries. I don't think it's feasible for an app developer to determine which libraries.
Given that this class seems to have been specifically designed to crash in the worst way possible for the worst reason possible, I'm going to reopen this to figure out what the best course of action is going to be here. It may be best to simply deprecate this class and avoid using it in any Jetpack libraries.
yb...@google.com <yb...@google.com> #11
i think it is time to remove that cache. it is really not useful based on our micro benchmarks anymore anyways.
yb...@google.com <yb...@google.com> #12
measurements for reference: (sorry for the internal links)
tl;dr; it does get slightly slower for the micro benchmark yet that is basically the best case for this cache and even that does not matter much.
While there is a very small increase of number of objects / bytes with the
cache-less version of SimpleArrayMap, remember this is the memory footprint of
the entire test suite (3 individual tests using a SimpleArrayMap of 1000 key-
value pairs each), the delta (208 bytes for 4 more objects) is very small
relative to the number of objects involved.
al...@google.com <al...@google.com> #13
I am very okay with removing the cache given the volume of basically-unresolvable issues that it has caused.
sj...@disneystreaming.com <sj...@disneystreaming.com> #14
au...@google.com <au...@google.com> #15
Currently nobody is working on it, but we'd be happy to take your patch
yb...@google.com <yb...@google.com> #16
I think we are already removing the problematic cache as part of the kotlin migration.
dustin cc'ed to verify.
du...@google.com <du...@google.com> #17
I can confirm we're removing the global array cache.
sj...@disneystreaming.com <sj...@disneystreaming.com> #18
ap...@google.com <ap...@google.com> #19
Branch: androidx-main
commit 4e996bf3fc80bb378714d62c9e2308ad10708466
Author: Dustin Lam <dustinlam@google.com>
Date: Mon Mar 14 12:02:46 2022
Remove global array cache from ArraySet
This cache provides dubious performance value, while causing a bunch of
separate headaches from the linked bugs.
Bug: 139401479
Bug: 182813986
Test: ./gradlew collection:collection:test
Change-Id: I36671e94753e6e1a8ccd5b513931614d1235202b
M collection/collection/src/jvmMain/java/androidx/collection/ArraySet.java
ap...@google.com <ap...@google.com> #20
Branch: androidx-main
commit f3ccc7d4febe58d0b861039d5fd0dd45ec00ad41
Author: Dustin Lam <dustinlam@google.com>
Date: Fri Mar 04 10:34:54 2022
Convert SimpleArrayMap to Kotlin
This also removes the global array cache which provided dubious
performance value while allowing unsychronized access to possibly
corrupt other instances of this class.
Relnote: "Convert SimpleArrayMap to Kotlin. This change introduces a few
incompatible changes, as a result of Java-Kotlin interop and the
ability to correctly define nullity of types in the source.
* The package private APIs, `.mSize`, `.mArray`, `.mHashes`,
`.indexOf()`, `.indexOfNull()`, and `.indexOfValue()`, were made
private - this is technically a binary incompatible change, but
reflects the intended visibility of these fields and is the closest we
can achieve in Kotlin since it does not include a way to specifiy
package-private visibility.
* The nullity of some types are now properly defined, the affected
methods are: `.getOrDefault`, `.keyAt`, `.valueAt`, `.setValueAt`,
`.put`, `.putIfAbsent`, `.removeAt`, `.replace`.
* For Kotlin users, `.isEmpty()` is now only available as a function instead of
also through property access."
Bug: 139401479
Bug: 182813986
Test: ./gradlew bOS
Change-Id: I271b70587e94ef166be71c5b60d8c6361b4b1849
M collection/collection/api/current.ignore
M collection/collection/api/current.txt
M collection/integration-tests/testapp/src/main/kotlin/androidx/collection/integration/SimpleArrayMapKotlin.kt
M collection/collection/api/public_plus_experimental_current.txt
M collection/collection/api/api_lint.ignore
M collection/collection/src/jvmMain/java/androidx/collection/ArrayMap.java
M collection/integration-tests/testapp/src/main/java/androidx/collection/integration/SimpleArrayMapJava.java
M collection/collection/api/restricted_current.txt
M collection/collection/api/restricted_current.ignore
M collection/collection/src/jvmMain/kotlin/androidx/collection/SimpleArrayMap.kt
Description
Artifact used Happening across every version. Devices/Android versions reproduced on: All versions.
This is our biggest issue on our Crashlytics dashboard. It has been ongoing for months.
Sample stacktraces:
Fatal Exception: java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Object[] at androidx.collection.SimpleArrayMap.allocArrays(:170) at androidx.collection.SimpleArrayMap.put(:458) at androidx.coordinatorlayout.widget.DirectedAcyclicGraph.addNode(:55) at androidx.coordinatorlayout.widget.CoordinatorLayout.prepareChildren(:698) at androidx.coordinatorlayout.widget.CoordinatorLayout.onMeasure(:767) at android.view.View.measure(View.java:25785)
RecyclerView:
androidx.collection.SimpleArrayMap.allocArrays (Unknown Source:170) androidx.recyclerview.widget.RecyclerView.onLayout (Unknown Source:4404)
Bottom Navigation:
Fatal Exception: java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Object[] at androidx.collection.SimpleArrayMap.allocArrays(:170) at androidx.collection.SimpleArrayMap.put(:458) at androidx.transition.Transition.addViewValues(:1532) at androidx.transition.Transition.captureHierarchy(:1627) at androidx.transition.Transition.captureHierarchy(:1650) at androidx.transition.Transition.captureHierarchy(:1650) at androidx.transition.Transition.captureHierarchy(:1650) at androidx.transition.Transition.captureValues(:1511) at androidx.transition.TransitionManager.sceneChangeSetup(:318) at androidx.transition.TransitionManager.beginDelayedTransition(:418) at com.google.android.material.bottomnavigation.BottomNavigationMenuView.updateMenuView(:590) at com.google.android.material.bottomnavigation.BottomNavigationPresenter.updateMenuView(:69) at androidx.appcompat.view.menu.MenuBuilder.dispatchPresenterUpdate(:292) at androidx.appcompat.view.menu.MenuBuilder.onItemsChanged(:1063) at androidx.appcompat.view.menu.MenuBuilder.startDispatchingItemsChanged(:1090) at androidx.appcompat.view.menu.MenuBuilder.setExclusiveItemChecked(:627) at androidx.appcompat.view.menu.MenuItemImpl.setChecked(:622) at com.google.android.material.bottomnavigation.BottomNavigationMenuView$1.onClick(:128) at android.view.View.performClick(View.java:6291) at android.view.View$PerformClick.run(View.java:24931) at android.os.Handler.handleCallback(Handler.java:808) at android.os.Handler.dispatchMessage(Handler.java:101) at android.os.Looper.loop(Looper.java:166) at android.app.ActivityThread.main(ActivityThread.java:7529) at java.lang.reflect.Method.invoke(Method.java) at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:245) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:921)
There are many different variations, but they all have one thing in common:
androidx.collection.SimpleArrayMap
It has happened to me on occasion, there's no pattern to it. Sometimes I'll just start the app, press a button on the BottomNavigationView then it crashes with
java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Object[]
To be clear, we are not using
SimpleArrayMap
in our code, this is coming from within the Framework. There are several cases in our dashboard, mostly stemming from View methodsmeasure
,layout
,performClick
,dispatchOnPreDraw
, etc.