[TIMOB-28516] Android: Setting ScrollableView "views" property after window open slower as of 10.0.1
GitHub Issue | n/a |
---|---|
Type | Bug |
Priority | Critical |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2021-08-09T09:24:41.000+0000 |
Affected Version/s | Release 10.0.1 |
Fix Version/s | Release 10.0.2 |
Components | Android |
Labels | ScrollableView, android, performance, regression |
Reporter | Hans Knöchel |
Assignee | Joshua Quick |
Created | 2021-08-02T09:52:03.000+0000 |
Updated | 2021-08-09T09:24:41.000+0000 |
Description
We noticed an increased lack of performance (on the UI thread) when using SDK 10.0.1 and beyond. The issue is reflected by the following log (shown > 25 times due to our high usage of card views):
[INFO] MaterialCardView: Setting a custom background is not supported.
Our main list (uses card views a lot), which is inside a scrollable view. So could this maybe be a regression from the recent changes in the Ti.UI.ScrollableView?
It can be easily reproduced in our app by using a 10_0_X build vs 10.0.0.GA and adding a new POI inside a waypoint. Usually, it redraws in 1-2 seconds (by the socket update), but now it takes up to 8 seconds due to the UI thread being blocked.
[~hknoechel], your app's startup time does not seem any slower between Titanium 10.0.0, 10.0.1, and 10.1.0. A cold-start of your app takes about
5 seconds
on my device for the "debug" version, which is what the CLI builds when the deployment type is "test" or "development". In your "build.gradle", I can disable debugging in the debug build (I know that sounds weird) via the following "build.gradle" setting and the launch time is about2 seconds
, which is about the same time your release version on Google Play takes.So I think it's just the traces/debug messages that Google outputs that's making it take longer. Also, the "debuggable" setting makes the Android OS check for private API usage and probably some other things.
Also the MaterialCardView warning won't cause this performance issue. Looking at Google's code, it simply logs that message and replaces the color drawable. We can override this behavior when setting the "backgroundColor" to get rid of this warning message, but it has nothing to do with the performance issue you saw. https://github.com/material-components/material-components-android/blob/master/lib/java/com/google/android/material/card/MaterialCardView.java#L362-L371
How expensive are the
getViews()
calls in https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIScrollableView.java ? Eg. inonPageScrollStateChanged
there are 3, inonPageScrolled
are 4; before it was justmViews
The
getViews()
just returns the collection reference as-is. So, there's no issue there. https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIScrollableView.java#L379 https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/ScrollableViewProxy.java#L139This really worries me. @Josh: Can you go ahead and add a waypoint on the waypoint map and compare the time between the pin is drawn between 10.0.0 and 10.0.2? We literally swapped SDK versions and saw a difference of 4x in redraw performance
I tested your app with your "hans2" account. On my Pixel 3a device running Android 11 the app startup times are... * Titanium [PR #12989](https://github.com/appcelerator/titanium_mobile/pull/12989) - debug build:
5 seconds
* Titanium [PR #12989](https://github.com/appcelerator/titanium_mobile/pull/12989) - release build:2 seconds
* Titanium 10.0.0.GA - debug build:5 seconds
* Titanium 10.0.0.GA - release build:2 seconds
On my Android 12 emulator, the app startup times are... * Titanium [PR #12989](https://github.com/appcelerator/titanium_mobile/pull/12989) - debug build:13 seconds
* Titanium [PR #12989](https://github.com/appcelerator/titanium_mobile/pull/12989) - release build:11 seconds
* Titanium 10.0.0.GA - debug build:13 seconds
* Titanium 10.0.0.GA - release build:11 seconds
So, I'm not seeing a difference in app startup times between 10.0.0.GA and the Titanium SDK I've built with [PR #12989](https://github.com/appcelerator/titanium_mobile/pull/12989). The emulator startup times are pretty bad and I'm not sure why yet, but on a real device the startup times are good.I'm able to reproduce this performance issue. It's caused by setting the "views" property after the window has opened. As of 10.0.1, we release all of the native views in the last "views" array before assigning it new views. This would normally be okay except when the new array contains the same views as before in which case we don't want to release them (this is what [~hknoechel] is running into). Also we're notifying the native view adapter for every view in the array when we should only invoke this notification once after adding all views.
Regarding the CardView warning message, it's harmless. We'll look into resolving it via ticket [TIMOB-28517] which is a good opportunity to support "touchFeedbackColor" which has always been ignored before.
PR (master): https://github.com/appcelerator/titanium_mobile/pull/13001
The issue is now fixed - brilliant work, Josh, thank you!
Merged to master and 10_0_X