Reputation: 22637
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
?
It's calling onBindViewHolder()
for every element in the list. This doesn't seem correct.
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
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
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.
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.
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).
@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(...)
}
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.
@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.
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
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