1

I display a list of device's contacts. In my Samsung Galaxy S8 with Android version 9, when I scroll the recyclerView for very first time, it is not smooth and lagging a little bit. But then it start to scroll smoothly very well. If I close the app using back button and start the app again, it scrolls smoothly again, but If I destroy instance of app from recent history, and start the app again, it is not smooth and lagging a little bit just at first scroll. ( I have tested in Google Pixel2, and when there is low buttery, I feel the same lag as I explained. )

Here is a recorded screen from the issue in Galaxy s8 : https://drive.google.com/file/d/1szfF1oKEYZK3LIqQHC-MsapRxdJ85bFy/view?usp=sharing

I optimized recyclerView adapter as much as possible, and it seems issue is not related to my Adapter. You can check the source code here : https://github.com/AliRezaeiii/DignityContacts

I have a customized CoordinatorLayout.Behavior to hide/show AppBarLayout as well as bottomBar. I am sure that it is not related to that since when there is not CoordinatorLayout behavior, it shows lagging again as I explained above.

Here is my recyclerView item layout :

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" android:layout_width="match_parent" 
    android:layout_height="wrap_content" 
    android:background="@android:color/white" 
    android:orientation="vertical">
    
    <include layout="@layout/contact_separator" />
    
    <include layout="@layout/contact_detail" />
    
    <LinearLayout
        android:id="@+id/subItem"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:orientation="vertical" />
    
    </LinearLayout>

contact_separator and contact_detail are both ConstaintLayouts.

Here is how I setup recyclerView :

mAdapter = new ContactsAdapter();
binding.recyclerView.setHasFixedSize(true);
binding.recyclerView.setAdapter(mAdapter);

final Observer<Resource<List<Contact>>> contactsObserver = resource -> {
            if (resource instanceof Resource.Success) {
                mContacts = ((Resource.Success<List<Contact>>) resource).getData();
                mAdapter.setItems(mContacts, true);
            }
        };

What could be the reason of that?

Ali
  • 9,800
  • 19
  • 72
  • 152

2 Answers2

1

It's difficult to tell without the project running, but a very quick look (as quick as 2-3 minutes browsing through your Adapter code), there are some "smells".

  1. You're using a plain RecyclerView.Adapter, you should/could be using ListAdapter<T, K> with a DiffUtilCallback to avoid using notifyDataSetChanged() when you do setList(...) (instead you only have to do submitList(...) and let the Adapter resolve the problem for you.

  2. Your bind(...) method is quite complex and long; there's two potential loops and view-param change (widget dimensions), this would cause at least a measure pass (which would be followed by a layout pass), all this happens for each item in the view holder, when you submit list. When the app is created (after it was being destroyed), all this must be computed (again) so the recycler view, which has a setFixedSize = true (why do you need this, this is not true given your adapter). If fixed Size is true, then a requestLayout() call is avoided but this means you're telling your adapter its size is constant (meaning it can grow/shrink based on item count, but all the items have the same size). It's an optimization, but it can bite you back and it's most likely not what you want.

  3. Your flagItems is a LinearLayout, you add/remove views to this linear layout dynamically at runtime, inside the bind method.

  4. You do the same for subItem (another LinearLayout).

  5. Then there's again the possibility of another loop (Contact numbers) where the layout is potentially modified again (Constraint sets are changed).

All this (there are some other details) but this would do for now... are "red flags" when it comes to Adapters.

The fun part is that all this should happen in less time that it takes android to render a frame (60ms?). So you are asking the code to do a lot of work and triggering a lot of side-effects, and expect it to do it in 60ms or less. This for EACH view that is getting bound (depending on the device's screen size), so imagine 5 items fit on screen; the RecyclerView will bind a few more in advance to be prepared for when you scroll.

What can you do?

I'd start by taking a step back at your List<Contact> contacts. These Contacts could be better transformed for presentation on a RecyclerView. All the logic about what/how to show it, should be resolved ahead of time, and presented to the adapter (Which already has other work to do) in the flattest/simplest possible form. You could use the type of ViewHolders to your advantage, by dissecting your data into different types and letting the adapter simply bind the correct viewHolder for each view type.

So instead of a List<Contact> you'd supply a List<SomeOtherObject> which is better suited for the viewHolder, and only contains the data you'd need to later re-obtain the original Contact (if needed), or to reconstruct it.

Then you can simplify and remove all that logic/decision making from the viewHolder#bind method, since a lot of that is resolved when you determine which type of data you are going to bind.

I'd start there, because you can try to optimize all you want, but you're still asking the Adapter to do all this work for you.

Why does this NOT happen when you "don't kill the app"?

That's because (I estimate) the things are cached and pre-calculated (they already took time the 1st time), so because you have FixedSize = true, the RecyclerView knows (has been told to) that the size of each ViewHolder will not change, so there's no need to re-calculate it.

In short, you didn't make your adapter more performant, you simply told him to spend that amount of time once. ;)

Martin Marconcini
  • 26,875
  • 19
  • 106
  • 144
  • Thank you for reviewing the codes, analyze them and a great suggestion about using type of ViewHolders instead of my complex logic in bind method. If I do not set FixedSize as true, lagging might happen everytime. Do you agree? – Ali Feb 08 '21 at 09:09
  • Yeah, but If you have dynamic sized viewholders (where the content will/may affect the size of the view), then you *cannot* set it to true, otherwise your views will be all the same size (the ViewHolder height/width) regardless of the content and the content will be clipped. – Martin Marconcini Feb 08 '21 at 09:10
  • I set FixedSize to false but again lagging happens at very first time and when I kill the app, although your description about that seems logical. So I have no idea about the reason. – Ali Feb 08 '21 at 12:35
  • Try commenting things from your `bind` method. Like only bind "one text" and comment EVERYTHING else. Does this perform fast? Where (in the Lifecycle) are you setting the data? – Martin Marconcini Feb 08 '21 at 14:15
  • I commented everything and strangely I feel the lagging at first scroll again in Galaxy s8. I have a liveData in my ViewModel which I observe in `onCreateView`, and set the data when result is delivered. You can check the code at the end of my question. – Ali Feb 08 '21 at 14:24
  • ok, what if you provide a static list (like `setList(listOf(...)` once (just hardcode a few items, since you're only binding a "text" you don't need to provide much. Add a Constructor to your data model so you can do `listOf(Contact("one"), Contact("two"), etc.)` :) Otherwise, there may be another issue with your Activity/Fragment that is causing the message queue to block... this eliminates the data fetching from the equation. – Martin Marconcini Feb 08 '21 at 14:42
  • Static list have the issue as well. `onCreateViewHolder` get called few more times at very first scroll as you mentioned in your answer, and that could explain why I feel the lagging at first. Although if I kill the app or restart my Activity, `onCreateViewHolder` get called almost same amount of time as I debugged. Could it be possible that when we restart the Activity (not kill the app) things still are cached and pre-calculated? – Ali Feb 09 '21 at 22:42
  • You mentioned : **the RecyclerView will bind a few more in advance to be prepared for when you scroll** which is correct, but it called same amount of time when I kill the app or click on back button and start the app again. Are things still cached and pre-calculated? – Ali Feb 09 '21 at 22:46
  • I just researched a little bit and found [this](https://stackoverflow.com/questions/1977246/android-finish-method-doesnt-clear-app-from-memory) : activity is not cleared from memory by clicking on back button and as Romain Guy said : **Android keeps processes around in case the user wants to restart the app, this makes the startup phase faster.** Could it be the reason? I will be happy to hear your opinion. – Ali Feb 10 '21 at 18:05
  • It could be, I'm just now curious to see what part, because if you have a recycler view, whose ViewHolder is "just a text view" and the data is just a list of "Things" which are very simple and contain a single "String" to bind, then there's no reason for this to be "slow", this is how we construct RecyclerViews since the age of dawn, that's why I was asking you to use a simple static list of "x" elements and a very simple viewHolder to rule out other issues. – Martin Marconcini Feb 11 '21 at 10:28
  • As for what Romain said, yes, Activities aren't necessarily always destroyed but their views are most likely and hence why you see the onCreateView regardless. That being said, you probably have another issue that is causing the slowdown, my bet is something is blocking (or taking long time) to complete during this initialization. Do you see any "Choreographer" errors in the Log? – Martin Marconcini Feb 11 '21 at 10:30
  • I see : **Choreographer: Skipped 57 frames! The application may be doing too much work on its main thread.** in my emulator with API level 27, but not my Pixel 2 device with API 30, and emulator with API level 23. It seems `Choreographer` error happens but not in all APIs. At least I am sure it happens in API level 27 and 28, but not in API level 22, 23 and 30. That explains why I feel the lagging. Thank you @Martin Marconcini, you were really helpful. – Ali Feb 11 '21 at 12:31
  • No worries, yeah, you must be blocking the main thread too much; consider offloading all the fetching, transformation, and preparation of the List to pass onto the adapter to a coroutine running on another context/thread and only push the liveData back to the Fragment once you're done, this way your adapter creation/etc. is done in onCreateView, but the actual data is put in the adapter after it's finished being computed/created in another thread. (I didn't look into your project in full detail, perhaps you're already doing some of this) – Martin Marconcini Feb 11 '21 at 12:36
  • I do all the heavy calculations in background using RxJava : [this](https://github.com/Ali-Rezaei/Contacts/blob/master/app/src/main/java/com/sample/android/contact/util/ContactUtils.java) and [this](https://github.com/Ali-Rezaei/Contacts/blob/master/app/src/main/java/com/sample/android/contact/repository/ContactsRepository.kt). I feel my UI xmls are very complex which is called in `onCreateViewHolder` and that could be why I see `Choreographer` errors when RecyclerView is loading and at the first scroll. Correct me if I am wrong. – Ali Feb 11 '21 at 12:59
  • Yeah, I'd say that if your XML is complex, then it may take too much time (>60ms) to keep up with the 60fps motto. I suggest breaking down your hierarchy into a flatter/simpler structure. There's little need to use nested linear layouts (or to nest too many things) You can always experiment with the ` – Martin Marconcini Feb 11 '21 at 13:36
  • +1 for your help. I agree, I think if I use type of ViewHolders by `getItemViewType`, it may solve my problem. – Ali Feb 11 '21 at 17:36
  • Thanks :-) If you need a sample (unrelated) I've done this type of RecyclerView since the age of dawn, here's a simple example where there are 3 different types in one "convenient" file. [The project is hosted in github](https://github.com/Gryzor/GridToShowAds/blob/master/app/src/main/java/com/neutobo/recyclerviewwithads/ThingAdapterWithAds.kt). Enjoy! :) – Martin Marconcini Feb 12 '21 at 08:44
  • I simplified the bind method in my [adapter](https://github.com/Ali-Rezaei/LibertyContacts/blob/master/app/src/main/java/com/sample/android/contact/ui/ContactsAdapter.java), but issue still exist in a few android versions. I reported it as a bug here : https://issuetracker.google.com/issues/179553951 – Ali Mar 06 '21 at 11:01
  • I still see your adapter is "not simple" at all, look at those `bind` methods, they are iterating over stuff, and creating new layouts on the fly, inflating them, etc. That's still a lot of work and the reason why Choreographer will have no choice but to skip frames when it cannot complete the whole thing in a frame. – Martin Marconcini Mar 07 '21 at 10:31
0

You should initialize adapter with a list, currently you're setting an observer and when it gets observed then the list is initialised that why it's lagging.

Hussain
  • 81
  • 1
  • 6
  • I have a background job, and I need to wait until data is ready using observing LiveData. I can not imagine a better solution than submitList when data is delivered. How can I initialize adapter in advance? – Ali Feb 09 '21 at 22:53