Status Update
No update yet.
Comments
au...@google.com
Branch: androidx-master-dev
commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020
Semantics no-op cleanups
Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:
- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".
- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)
- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)
Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."
Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc
M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt
al...@google.com
This happens when the class is accessed on concurrently on different threads. The class itself -- not isolated instances of the class -- is intentionally not thread-safe as a trade-off for performance. This is not a good design choice, but that's what it inherited from the platform implementation.
So, any library or app that's calling the class without explicit synchronization will cause usages elsewhere (e.g. in a library) to crash.
This is almost certainly an issue with the app's own usage.
co...@gmail.com
Thanks for response. Can you ELI5 any solutions I can consider if it's going to be a won't fix?
I have started looking into all the other active threads listed in the Crashlytics logs to see if there could be concurrency issues. I have seen a few cases of stacktraces that lead to code in our app, and there's a trend; most of them are the loadInitial method from Paging library.
Paging has a strict synchronous loading rule, we need to ensure that our data returns before submitList is called. So could it be by design that Paging Library is the culprit?
co...@gmail.com
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
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
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
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
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
i think it is time to remove that cache. it is really not useful based on our micro benchmarks anymore anyways.
yb...@google.com
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
I am very okay with removing the cache given the volume of basically-unresolvable issues that it has caused.
sj...@disneystreaming.com
au...@google.com
Currently nobody is working on it, but we'd be happy to take your patch
yb...@google.com
I think we are already removing the problematic cache as part of the kotlin migration.
dustin cc'ed to verify.
du...@google.com
I can confirm we're removing the global array cache.
sj...@disneystreaming.com
ap...@google.com
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
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.SimpleArrayMapIt 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
SimpleArrayMapin 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.