Glide: Lagging behavious in recyclerview

Created on 15 Dec 2015  路  6Comments  路  Source: bumptech/glide

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.

question

Most helpful comment

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:

  • To reverse the 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.

  • you could extract a method into 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.
  • possible bug: in CardViewHolder you bind 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.

  • really minor: x.getType() != null && x.getType().equals("constant")
    is the same as "constant".equals(x.getType()), but no null-check and no NPE
  • childrenItemB != null && is redundant, former lines would have already thrown an NPE
  • Even though I said you can't use RecyclerView 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.
  • (warning: this may be premature!) You could try to minimize number of add/removes inside the card by making the recycle smarter :

    • don't release and remove all views

    • just before the fchildb.getChildrenItemBs() make sure there are enough views in ll_hsv:

    • if there are exactly the same number of children (k) as itemBs (n): don't do anything

    • if there are fewer (k < n): add n - k items

    • if there are more (k > 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.

  • You didn't post your XML, so make sure you're not using wrap_content on the ImageViews as it will load an unnecessarily big (size of the screen) image. match_parent and fixed dp is fine.

All 6 comments

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:

  • To reverse the 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.

  • you could extract a method into 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.
  • possible bug: in CardViewHolder you bind 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.

  • really minor: x.getType() != null && x.getType().equals("constant")
    is the same as "constant".equals(x.getType()), but no null-check and no NPE
  • childrenItemB != null && is redundant, former lines would have already thrown an NPE
  • Even though I said you can't use RecyclerView 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.
  • (warning: this may be premature!) You could try to minimize number of add/removes inside the card by making the recycle smarter :

    • don't release and remove all views

    • just before the fchildb.getChildrenItemBs() make sure there are enough views in ll_hsv:

    • if there are exactly the same number of children (k) as itemBs (n): don't do anything

    • if there are fewer (k < n): add n - k items

    • if there are more (k > 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.

  • You didn't post your XML, so make sure you're not using 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?)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sant527 picture sant527  路  3Comments

MrFuFuFu picture MrFuFuFu  路  3Comments

kenneth2008 picture kenneth2008  路  3Comments

Anton111111 picture Anton111111  路  3Comments

Tryking picture Tryking  路  3Comments