Hi, this is not the issue I think.
I am using glide to loading images and my list is very huge. so Its lagging when I am scrolling. So could you please suggest me how can I use glide to avoid this problem.
Thanks in advance.
The length of the list usually doesn't matter, that's why we have recycling adapters (to bind only 3-4 items at once). Although if you have linear operations, scrolling the list becomes quadratic (slow), what is the data structure backing the list and what do you do with it?
The area of this list shouldn't matter as well, because we usually have 1 list per full screen (+ maybe some top-bottom toolbars/tabs).
Based on the above two and the async nature of Glide it shouldn't be lagging. Excessive re-layout can cause lags, try to disable Glide to see if it happens without.
Can you please describe your problem in a little more detail? Also add adapter (mostly onBindVH), Glide load line and layout XML for list items. Maybe a screenshot (you can blur private stuff) or an adb screenrecord. And in general stuff that may be relevant, see issue template for examples.
Thanks to quick reply. I am reviewing my code. as you asked me. I added onBindVH code below
Fchildb fchildb = fchildbList.get(position);
if (holder instanceof ProgressViewHolder) {
((ProgressViewHolder) holder).progressBar.setVisibility(View.VISIBLE);
} else if (holder instanceof PagerViewHolder) {
if (fchildb != null && fchildb.getType().equals("pager")) {
ArrayList<ChildrenItemB> childrenItemBs = new ArrayList<>();
try {
JSONArray jsonArray = new JSONArray(fchildb.getChildren());
for (int i = 0; i < jsonArray.length(); i++) {
ChildrenItemB childrenItemB = RestResponse.parseChildren(jsonArray.get(i).toString());
childrenItemBs.add(childrenItemB);
}
} catch (JSONException e) {
e.printStackTrace();
}
SlidingPageAdap pageAdap = new SlidingPageAdap(ctxt, childrenItemBs);
((PagerViewHolder) holder).viewPager.setAdapter(pageAdap);
} else {
LinearLayout.LayoutParams vpParams = new LinearLayout.LayoutParams(0, 0);
((PagerViewHolder) holder).viewPager.setLayoutParams(vpParams);
// holder.viewPager.setVisibility(View.GONE);
}
} else if (holder instanceof CardViewHolder) {
try {
if (fchildb.getType().equals("c") || fchildb.getType().equals("b"))
((CardViewHolder) holder).tv1.setText(fchildb.getCname());
else
((CardViewHolder) holder).tv2.setText(fchildb.getT());
((CardViewHolder) holder).tv3.setTag(fchildb.getCid());
JSONArray jsonArray = new JSONArray(fchildb.getChildren());
for (int i = 0; i < jsonArray.length(); i++) {
final ChildrenItemB childrenItemB = RestResponse.parseChildren(jsonArray.getJSONObject(i).toString());
View view = LayoutInflater.from(ctxt).inflate(R.layout.item_horizontal_cardview, null, false);
TextView tv_name = (TextView) view.findViewById(R.id.tv_name);
TextView tv_mktp = (TextView) view.findViewById(R.id.tv_mktP);
TextView tv_ourp = (TextView) view.findViewById(R.id.tv_ourP);
tv_n.setText(childrenItemB.getT().toString());
if (childrenItemB.getType() != null && childrenItemB.getType().equals("c")) {
tv_mktp.setText(childrenItemB.getCname());
} else {
if (Float.parseFloat(childrenItemB.getMkt_p()) > 0) {
tv_mktp.setText("Rs. " + childrenItemB.getMkt_p());
}
tv_ourP.setText("Rs. " + childrenItemB.getOur_p());
}
ImageView imv_item = (ImageView) view.findViewById(R.id.imv_item);
if (childrenItemB != null && childrenItemB.getImg_url() != null) {
String url = "";
MI.loadImage(ctxt, url, imv_item);
}
view.setTag(childrenItemB);
((CardViewHolder) holder).ll_hsv.addView(view, i);
}
} catch (JSONException e) {
e.printStackTrace();
}
}
and this is my loadImage method
public static void loadImage(Context ctxt,String url,ImageView imv){
Glide.with(ctxt).load(url).placeholder(R.mipmap.ic_launcher).into(imv);
}
Please suggest and improve me If I am doing wrong.
try to disable Glide to see if it happens without
What I meant here is to comment this line and see if it still lags:
//MI.loadImage(ctxt, url, imv_item);
Based on my initial hunches above and the below discussion I'll close this issue, as it's most likely not Glide related.
Excessive re-layout can cause lags
What I was referring to here is exactly what you're doing with CardViewHolder: not reusing the views. The whole idea of the View Holder pattern is to prevent continuous inflate and findViewById calls. You must be clearing CardViewHolder.ll_hsv somewhere, probably in onViewRecycled or before the definition of fchildb, otherwise that list inside the card would grow indefinitely. This means that on every card bind you're inflating a lot of views inside the card. This is not advised and will cause lags, becuase it takes a lot of time and likely causes continuous layout as well. Below is an attempt to remove this from your code by pooling the list items inside card. Note that you can't use a RecyclerView or ListView inside the card, because it would just create all those views as well. This solution gives one pool across all cards, so it prevents inflates. It'll still cause layouts, because of removeAllViews and add calls.
class MyAdapter extends RecyclerView.Adapter<RecyclerView.ViewHolder> {
private CardListPool cardItemsPool;
@Override public void onBindViewHolder(ViewHolder holder, int position) {
if (holder instanceof CardViewHolder) {
CardViewHolder cardHolder = ((CardViewHolder)holder);
// bind cardHolder.tv1/tv2/tv3 as usual
for (ChildrenItemB childB : fchildb.getChildrenItemBs()) {
CardItemBHolder item = cardItemsPool.acquireOrInflate();
cardHolder.ll_hsv.add(item.itemRoot);
item.bind(childB);
}
}
}
@Override public void onViewRecycled(ViewHolder holder) {
if (holder instanceof CardViewHolder) {
ViewGroup cardList = ((CardViewHolder)holder).ll_hsv;
for (int i = 0, count = cardList.getChildCount(); i < count; ++i) {
cardItemsPool.tryRelease(cardList.getChildAt(i));
}
cardList.removeAllViews();
}
}
static class CardItemBHolder { // notice it's not a RecyclerView.ViewHolder
View itemRoot;
TextView tv_name;
TextView tv_mktP;
TextView tv_ourP;
ImageView imv_item;
ChildrenItemB boundChild;
CardItemBHolder(View view) {
view.setTag(this);
this.itemRoot = view;
this.tv_name = (TextView)view.findViewById(R.id.tv_name);
this.tv_mktP = (TextView)view.findViewById(R.id.tv_mktP);
this.tv_ourP = (TextView)view.findViewById(R.id.tv_ourP);
this.imv_item = (ImageView)view.findViewById(R.id.imv_item);
}
public void bind(ChildrenItemB child) {
// TODO (ChildrenItemB)view.getTag() -> ((CardItemBHolder)view.getTag()).boundChild
boundChild = child;
tv_name.setText(child.getT().toString());
if (child.getType().equals("c")) {
tv_mktP.setText(child.getCname());
} else {
if (Float.parseFloat(child.getMkt_p()) > 0) {
tv_mktP.setText("Rs. " + child.getMkt_p());
}
tv_ourP.setText("Rs. " + child.getOur_p());
}
// Glide can handle null url, see .fallback()
MI.loadImage(imv_item.getContext(), child.getImg_url(), imv_item);
}
public void unbind() {
boundChild = null;
tv_name.setText(null);
tv_mktP.setText(null);
tv_ourP.setText(null);
Glide.clear(imv_item);
}
}
static class CardListPool extends android.support.v4.util.Pools.SimplePool<CardItemBHolder> {
private final LayoutInflater inflater;
public CardListPool(Context context) {
super(10 * 3); // magic constant assuming an Fchildb has around 10 itemBs max,
// and prepare for 3 Fchildbs to be bound at once
// (one visible center screen, two partially visible top/bottom screen)
// if it has more than than that it'll still work, a little slower and extraneous items will be discarded
inflater = LayoutInflater.from(context);
}
public CardItemBHolder acquireOrInflate() {
CardItemBHolder holder = super.acquire();
if (holder == null) {
View view = inflater.inflate(R.layout.item_horizontal_cardview, null, false);
holder = new CardItemBHolder(view);
}
return holder;
}
public boolean tryRelease(View view) {
Object tag = view.getTag();
if (tag instanceof CardItemBHolder) {
release((CardItemBHolder)tag);
return true;
} else {
return false;
}
}
@Override public boolean release(CardItemBHolder holder) {
holder.unbind();
return super.release(holder);
}
}
}
Other unrelated, but potentially useful suggestions:
instanceof antipattern you could move each of these else if branches into their respective ViewHolder classes by creating an abstract base class for these 3 VHs:java
public class MyAdapter extends RecyclerView.Adapter<MyAdapter.BaseViewHolder> {
static class BaseViewHolder extends RecyclerView.ViewHolder {
BaseViewHolder(View view) { super(view); }
abstract void bind(Fchildb fchildb);
}
static class CardViewHolder extends BaseViewHolder {...}
}
and in the adapter just call holder.bind(fchildbList.get(position)) in the adapter to dispatch. This will result that you can replace all ((MyViewHolder) holder) with this (or just omit it), making the code more readable.
Fchildb to generate (childrenItemBs list) and use it in both pager and card holders. If that method exists it can also cache the result in a field inside so it's not re-generated every time the user may scroll back and forth. It'll also halve the length of this massive bind method. If you can't modify Fchildb class then create an FchildbSupport class with static methods.tv1 and tv2 in different if branches, if an list item is reused between a type "b or c" object and something else one of the tvs will contain an irrelevant value, consider:java
boolean cOrB = fchildb.getType().equals("c") || fchildb.getType().equals("b");
tv1.setText(cOrB? fchildb.getCname() : null);
tv2.setText(cOrB? null : fchildb.getT());
or leave the current structure, but add tv1/2.setText(null) to both branches.
x.getType() != null && x.getType().equals("constant")"constant".equals(x.getType()), but no null-check and no NPEchildrenItemB != null && is redundant, former lines would have already thrown an NPERecyclerView in each card it may be possible and effective: the other day I stumbled upon the method RecyclerView.setRecycledViewPool which shares the pool between lists. You can give that a try too.fchildb.getChildrenItemBs() make sure there are enough views in ll_hsv:k) as itemBs (n): don't do anythingk < n): add n - k itemsk > n): remove items in range: [n, k - n]Though it'll only help if the number of itemBs are usually the same. First try to see if just working around the inflation helps.
wrap_content on the ImageViews as it will load an unnecessarily big (size of the screen) image. match_parent and fixed dp is fine.Thanks robert its very helpful. I would say more than that. Thanks once again.
And you are awesome :) :+1: :+1: :+1: :+1: :+1: :+1: :+1: :+1:
@TWiStErRob I am using the ConstraintLayout with layout_constraintDimensionRatio (1:1) attribute and also using override method in my GridLayoutManager and still facing the lagging issue. If I replace the imageUrl with a drawable it's scrolling seamlessly.
Glide.with(context)
.load(imageUrl)
.diskCacheStrategy(DiskCacheStrategy.ALL)
.thumbnail(0.20f)
.error(R.color.colorDarkGray)
.fallback(R.color.colorDarkGray)
.centerCrop()
.override(devicewidth/3)
.into(imageView)
item_layout.xml
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:layout_constraintDimensionRatio="1:1">
<ImageView
android:layout_width="match_parent"
android:layout_height="match_parent"
android:adjustViewBounds="true"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
</androidx.constraintlayout.widget.ConstraintLayout>
@deepakkumardk I have a feeling you're using app:layout_constraintDimensionRatio wrong: https://medium.com/@sasanksunkavalli/android-constraintlayout-layout-constraintdimensionratio-33bd2293f34c
(attribute should be on ImageView and potentially one or both w/h should be 0dp?)
Most helpful comment
What I meant here is to comment this line and see if it still lags:
Based on my initial hunches above and the below discussion I'll close this issue, as it's most likely not Glide related.
What I was referring to here is exactly what you're doing with
CardViewHolder: not reusing the views. The whole idea of the View Holder pattern is to prevent continuousinflateandfindViewByIdcalls. You must be clearingCardViewHolder.ll_hsvsomewhere, probably inonViewRecycledor before the definition offchildb, otherwise that list inside the card would grow indefinitely. This means that on every card bind you're inflating a lot of views inside the card. This is not advised and will cause lags, becuase it takes a lot of time and likely causes continuous layout as well. Below is an attempt to remove this from your code by pooling the list items inside card. Note that you can't use aRecyclerVieworListViewinside the card, because it would just create all those views as well. This solution gives one pool across all cards, so it preventsinflates. It'll still cause layouts, because ofremoveAllViewsandaddcalls.Other unrelated, but potentially useful suggestions:
instanceofantipattern you could move each of theseelse ifbranches into their respective ViewHolder classes by creating an abstract base class for these 3 VHs:java public class MyAdapter extends RecyclerView.Adapter<MyAdapter.BaseViewHolder> { static class BaseViewHolder extends RecyclerView.ViewHolder { BaseViewHolder(View view) { super(view); } abstract void bind(Fchildb fchildb); } static class CardViewHolder extends BaseViewHolder {...} }and in the adapter just call
holder.bind(fchildbList.get(position))in the adapter to dispatch. This will result that you can replace all((MyViewHolder) holder)withthis(or just omit it), making the code more readable.Fchildbto generate (childrenItemBslist) and use it in both pager and card holders. If that method exists it can also cache the result in a field inside so it's not re-generated every time the user may scroll back and forth. It'll also halve the length of this massive bind method. If you can't modifyFchildbclass then create anFchildbSupportclass with static methods.tv1andtv2in differentifbranches, if an list item is reused between a type "b or c" object and something else one of thetvs will contain an irrelevant value, consider:java boolean cOrB = fchildb.getType().equals("c") || fchildb.getType().equals("b"); tv1.setText(cOrB? fchildb.getCname() : null); tv2.setText(cOrB? null : fchildb.getT());or leave the current structure, but add
tv1/2.setText(null)to both branches.x.getType() != null && x.getType().equals("constant")is the same as
"constant".equals(x.getType()), but no null-check and no NPEchildrenItemB != null &&is redundant, former lines would have already thrown an NPERecyclerViewin each card it may be possible and effective: the other day I stumbled upon the methodRecyclerView.setRecycledViewPoolwhich shares the pool between lists. You can give that a try too.fchildb.getChildrenItemBs()make sure there are enough views inll_hsv:k) as itemBs (n): don't do anythingk < n): addn - kitemsk > n): remove items in range:[n, k - n]Though it'll only help if the number of itemBs are usually the same. First try to see if just working around the inflation helps.
wrap_contenton theImageViews as it will load an unnecessarily big (size of the screen) image.match_parentand fixeddpis fine.