Jeffrey Blattman
Jeffrey Blattman

Reputation: 22637

RecyclerView hangs main thread with many elements

I have a RecyclerView with an adapter that is based on a List of items. When I have ~400 elements in the adapter, I see the main thread hang for about 1-2 seconds at the point when I notify the adapter that I've set new data.

I've method traced it and it's just busy deep in the view hierarchy (starts in ViewRootImpl.performTraversal()). This doesn't make sense, as I'd expect it only to be rendering the 10-ish elements that are actually visible which should be lightning fast.

Am I missing some aspect of RecyclerView?

Edit

It's calling onBindViewHolder() for every element in the list. This doesn't seem correct.

Edit

If I give my RecyclerView a static height, it solves the problem. Not a solution in itself but hopefully points in a direction.

Upvotes: 3

Views: 2767

Answers (3)

Jeffrey Blattman
Jeffrey Blattman

Reputation: 22637

Unfortunately I still don't completely understand the root cause of this. When I set the height of the RecyclerView to something fixed (e.g. "400dp") the problem resolved itself. Obviously though that isn't a proper solution but it got me started.

No arrangement of layouts I tried would solve the problem. I eventually converted everything to use ConstraintLayout, and after fiddling with layout widths and heights I found something that resolved the problem. It was related to using "0dp" for the height. Feels bad that I don't understand the root cause. I pasted the layout below in case it can help anyone.

Overall I am real lukewarm on ConstraintLayout. It's very verbose, and the constraint paradigm (vs. what I guess you'd call containment) doesn't feel natural. For example, needing to use the floating Group element to show and hide other logical groups, and using "chains" to center groups of elements.

The ironic thing is that everything was working fine with old school LinearLayout. I embarked on this as a learning exercise. You have to wonder what the real performance hit is for the average layout of using traditional LinearLayout + ListView vs. ConstraintLayout + RecyclerView.

<?xml version="1.0" encoding="utf-8"?>
<android.support.constraint.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
  xmlns:tools="http://schemas.android.com/tools"
  xmlns:app="http://schemas.android.com/apk/res-auto"
  android:layout_width="match_parent"
  android:layout_height="match_parent"
  android:gravity="center">

  <ProgressBar
    android:id="@+id/progress_bar"
    style="@android:style/Widget.Holo.ProgressBar.Large"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:layout_gravity="center"
    android:visibility="visible"
    app:layout_constraintTop_toTopOf="parent"
    app:layout_constraintBottom_toBottomOf="parent"
    app:layout_constraintLeft_toLeftOf="parent"
    app:layout_constraintRight_toRightOf="parent"
    />

  <android.support.constraint.Group
    android:id="@+id/content_layout"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:orientation="vertical"
    android:visibility="gone"
    tools:visibility="visible"
    app:constraint_referenced_ids="header,list_view"
    />

  <android.support.constraint.ConstraintLayout
    android:id="@+id/header"
    android:layout_width="0dp"
    android:layout_height="48dp"
    app:layout_constrainedHeight="true"
    android:background="@color/gray_20"
    app:layout_constraintTop_toTopOf="parent"
    app:layout_constraintBottom_toTopOf="@id/list_view"
    app:layout_constraintLeft_toLeftOf="parent"
    app:layout_constraintRight_toRightOf="parent"
    >

    <View
      android:id="@+id/device_image"
      android:layout_width="0dp"
      android:layout_height="0dp"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="1"
      app:layout_constraintLeft_toLeftOf="parent"
      app:layout_constraintRight_toLeftOf="@id/device_name"
      app:layout_constraintTop_toTopOf="parent" />

    <TextView
      android:id="@+id/device_name"
      android:layout_width="0dp"
      android:layout_height="wrap_content"
      android:text="@string/name_label"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="2"
      app:layout_constraintLeft_toRightOf="@id/device_image"
      app:layout_constraintRight_toLeftOf="@id/device_model"
      app:layout_constraintTop_toTopOf="parent" />

    <TextView
      android:id="@+id/device_model"
      android:layout_width="0dp"
      android:layout_height="wrap_content"
      android:text="@string/model_label"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="2"
      app:layout_constraintLeft_toRightOf="@id/device_name"
      app:layout_constraintRight_toLeftOf="@id/device_id"
      app:layout_constraintTop_toTopOf="parent" />

    <TextView
      android:id="@+id/device_id"
      android:layout_width="0dp"
      android:layout_height="wrap_content"
      android:text="@string/id_label"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="2"
      app:layout_constraintLeft_toRightOf="@id/device_model"
      app:layout_constraintRight_toRightOf="parent"
      app:layout_constraintTop_toTopOf="parent" />
  </android.support.constraint.ConstraintLayout>

  <android.support.v7.widget.RecyclerView
    android:id="@+id/list_view"
    android:layout_width="0dp"
    android:layout_height="0dp"
    android:fadingEdgeLength="@dimen/fadingEdgeLength"
    android:footerDividersEnabled="false"
    android:requiresFadingEdge="vertical"
    app:layout_constraintTop_toBottomOf="@id/header"
    app:layout_constraintBottom_toBottomOf="parent"
    app:layout_constraintLeft_toLeftOf="parent"
    app:layout_constraintRight_toRightOf="parent"
    />
</android.support.constraint.ConstraintLayout><?xml version="1.0" encoding="utf-8"?>
<android.support.constraint.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
  xmlns:tools="http://schemas.android.com/tools"
  xmlns:app="http://schemas.android.com/apk/res-auto"
  android:layout_width="match_parent"
  android:layout_height="match_parent"
  android:gravity="center">

  <ProgressBar
    android:id="@+id/progress_bar"
    style="@android:style/Widget.Holo.ProgressBar.Large"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:layout_gravity="center"
    android:visibility="visible"
    app:layout_constraintTop_toTopOf="parent"
    app:layout_constraintBottom_toBottomOf="parent"
    app:layout_constraintLeft_toLeftOf="parent"
    app:layout_constraintRight_toRightOf="parent"
    />

  <android.support.constraint.Group
    android:id="@+id/content_layout"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:orientation="vertical"
    android:visibility="gone"
    tools:visibility="visible"
    app:constraint_referenced_ids="header,list_view"
    />

  <android.support.constraint.ConstraintLayout
    android:id="@+id/header"
    android:layout_width="0dp"
    android:layout_height="48dp"
    app:layout_constrainedHeight="true"
    android:background="@color/gray_20"
    app:layout_constraintTop_toTopOf="parent"
    app:layout_constraintBottom_toTopOf="@id/list_view"
    app:layout_constraintLeft_toLeftOf="parent"
    app:layout_constraintRight_toRightOf="parent"
    >

    <View
      android:id="@+id/device_image"
      android:layout_width="0dp"
      android:layout_height="0dp"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="1"
      app:layout_constraintLeft_toLeftOf="parent"
      app:layout_constraintRight_toLeftOf="@id/device_name"
      app:layout_constraintTop_toTopOf="parent" />

    <TextView
      android:id="@+id/device_name"
      android:layout_width="0dp"
      android:layout_height="wrap_content"
      android:text="@string/name_label"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="2"
      app:layout_constraintLeft_toRightOf="@id/device_image"
      app:layout_constraintRight_toLeftOf="@id/device_model"
      app:layout_constraintTop_toTopOf="parent" />

    <TextView
      android:id="@+id/device_model"
      android:layout_width="0dp"
      android:layout_height="wrap_content"
      android:text="@string/model_label"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="2"
      app:layout_constraintLeft_toRightOf="@id/device_name"
      app:layout_constraintRight_toLeftOf="@id/device_id"
      app:layout_constraintTop_toTopOf="parent" />

    <TextView
      android:id="@+id/device_id"
      android:layout_width="0dp"
      android:layout_height="wrap_content"
      android:text="@string/id_label"
      app:layout_constraintBottom_toBottomOf="parent"
      app:layout_constraintHorizontal_weight="2"
      app:layout_constraintLeft_toRightOf="@id/device_model"
      app:layout_constraintRight_toRightOf="parent"
      app:layout_constraintTop_toTopOf="parent" />
  </android.support.constraint.ConstraintLayout>

  <android.support.v7.widget.RecyclerView
    android:id="@+id/list_view"
    android:layout_width="0dp"
    android:layout_height="0dp"
    android:fadingEdgeLength="@dimen/fadingEdgeLength"
    android:footerDividersEnabled="false"
    android:requiresFadingEdge="vertical"
    app:layout_constraintTop_toBottomOf="@id/header"
    app:layout_constraintBottom_toBottomOf="parent"
    app:layout_constraintLeft_toLeftOf="parent"
    app:layout_constraintRight_toRightOf="parent"
    />
</android.support.constraint.ConstraintLayout>

Upvotes: 1

Martin Marconcini
Martin Marconcini

Reputation: 27236

I see a few issues and I'll point them out in no particular order; although I haven't seen anything specific yet that may be causing your issue, as I write this answer, I may discover a few things, so let's analyze your code.

What's missing?

You didn't include more code, so I have no clue how you use this adapter, or how does your recyclerview layout is (is it contained inside another scroll view or nested scroll view?) These are important questions because it may affect how the Android rendering/layout engine is working.

The Layout

You're using a LinearLayout (kinda not needed here) with weights. These are not recommended unless you have a very valid reason; calculating the widget's sizes and what not, is expensive, and more so when there are images and bitmaps involved.

I recommend you switch to a ConstraintLayout and do the right thing. You will thank yourself and your layout will likely perform better (nothing in your current layout is impossible for ConstraintLayout, from what I see).

The Adapter

  • This is strange:
    @Override
    public void onClick(View v) {
      Listener l;
      if ((l = listener) != null) {
        mainHandler.post(() -> l.onItemClick(devices.get(getAdapterPosition())));
      }
    }

A click should always happen on the Main Thread, you don't need to post anything. I'd write that as:

if (listener instanceof Listener) { //already fails if L is null anyway) 
     listener.onItemClick(...)
}
  • In your onBind you perform a lot of lookups, you can save cycles if you cache these. E.g.:
Picasso.with(context).load(SetupAppUtils.getDeviceImageUrl(context, device)).into(image);

I have no clue what your SsetupAppUtils.getDeviceImageUrl method does, but it could be expensive, and you could save that locally in a Map if needed (lookup the URL once, reuse) (just an idea, impossible to tell without looking at the code)

  • You seem to have a lot of business logic embedded in your onBind method (lots of if statements, is usually a red-flag. Your adapter couldn't care less about this, it's just "adapting" the data from model -> view and should not make decisions and perform transformations if it can be avoided. In this case, I see you pass a Device... seems ok but you appear to be doing a lot of checks and transformations, remember this is going to happen for all your items once they are being bound..., every time the user scrolls and a new item needs to be bound, you perform all this work again, when all you want is to "rebind" the viewholder.

  • You're notifying a full change, which means a LOT for a recyclerview (all has to be re-measured and laid out again) because you're essentially telling the adapter: all the data has changed.

  void setDevices(List<Device> devices) {
    this.devices.clear();
    this.devices.addAll(devices);
    notifyDataSetChanged();
  }

It's very straightforwards to use a DiffUtil (included with the framework!) and totally recommended.

  • You override a method, but all the items have the same id:
  @Override
  public long getItemId(int i) {
    return 0;
  }

If you cannot provide a good Id, then don't override this. I'd return item[i].something() (check for null/empty lists before!). Also rename i to position because that's what it is.

Anything Else?

Ouside of these, I don't see anything super strange. I'd make a simple test: remove all the binding code you have there (all the code in onBind) and just put name.SetText(...) and see if you still see the "lag". If you don't, then you know some of these operations are taking longer than others.

Try with less items, see what happens; a recyclerview should be binding only the views that are visible +/- a few up/down, not 400 items at once. The only reason I can think of, is if your RecyclerView is inside a NestedScrollView or similar.

Upvotes: 2

Omar Beshary
Omar Beshary

Reputation: 318

400 items to be loaded once is too much , you should use Pagination mechanism .Google now introduce paging library as part of android Jetpack. Plus you will find plenty of article and tutorials discussing how it could be implemented in android.

Upvotes: -1

Related Questions