Android - Laggy ListView with ImageView, LRUcache and ViewHolder

Question

I have a ListView in which every row has some text and two ImageView: one is the same for every row, the other depends on the current item.

This is my adapter:

mArrayAdapter(Context context, int layoutResourceId, ArrayList<Exhibition>  data) {
    super(context, layoutResourceId, data);
    this.context = context;
    this.layoutResourceId = layoutResourceId;
    this.list = data;
    this.originalList = data;
    viewHolder = new ViewHolder();
    final int maxMemory = (int) (Runtime.getRuntime().maxMemory() / 1024);
    final int cacheSize = maxMemory / 8;

    mMemoryCache = new LruCache<String, Bitmap>(cacheSize) {
        @Override
        protected int sizeOf(String key, Bitmap bitmap) {
            return bitmap.getByteCount() / 1024;
        }
    };

}

@Override
@NonNull
public View getView(final int position, View convertView, @NonNull final ViewGroup parent) {
    View row;
    final Exhibition ex;
    if(convertView==null){
        row = LayoutInflater.from(getContext()).inflate(R.layout.row,parent,false);

        viewHolder.expand = (ImageView)row.findViewById(R.id.expand);
        row.setTag(viewHolder);
    }
    else {
        row = convertView;
        viewHolder = (ViewHolder)row.getTag();
    }
    ex = list.get(position);


    descr = (TextView)row.findViewById(R.id.descr);
    ttl = (TextView)row.findViewById(R.id.title);
    city = (TextView)row.findViewById(R.id.city);
    dates = (TextView)row.findViewById(R.id.dates);
    museum = (TextView)row.findViewById(R.id.location);
    header = (ImageView)row.findViewById(R.id.hd);

    ttl.setText(ex.name);
    descr.setText(ex.longdescr);
    museum.setText(ex.museum);
    city.setText(ex.city);

    final Bitmap bitmap = getBitmapFromMemCache(ex.key);
    if (bitmap != null) {
        header.setImageBitmap(bitmap);
    } else {
        header.setImageBitmap(ex.getHeader());
        addBitmapToMemoryCache(ex.key,ex.header);
    }


    SimpleDateFormat myFormat = new SimpleDateFormat("dd/MM/yyyy", Locale.ITALY);
    Date start = new Date(ex.getStart()), end = new Date(ex.getEnd());

    String startEx = myFormat.format(start);
    String endEx = myFormat.format(end);

    String finalDate = getContext().getResources().getString(R.string.ex_date, startEx, endEx);

    dates.setText(finalDate);

    viewHolder.expand.setId(position);

    if(position == selectedId){
        descr.setVisibility(View.VISIBLE);
        ttl.setMaxLines(Integer.MAX_VALUE);
        dates.setMaxLines(Integer.MAX_VALUE);
        museum.setMaxLines(Integer.MAX_VALUE);
        city.setMaxLines(Integer.MAX_VALUE);
    }else{
        descr.setVisibility(View.GONE);
        ttl.setMaxLines(1);
        dates.setMaxLines(1);
        museum.setMaxLines(1);
        city.setMaxLines(1);
    }

    viewHolder.expand.setOnClickListener(this.onCustomClickListener);

    return row;
}

public void setDescr(int p){
    selectedId = p;
}

public void setOnCustomClickListener(final View.OnClickListener onClickListener) {
    this.onCustomClickListener = onClickListener;
}

public void addBitmapToMemoryCache(String key, Bitmap bitmap) {
    if (getBitmapFromMemCache(key) == null) {
        mMemoryCache.put(key, bitmap);
    }
}

public Bitmap getBitmapFromMemCache(String key) {
    return mMemoryCache.get(key);
}


@Override
public int getCount()
{
    return list.size();
}

@Override
public boolean isEnabled(int position)
{
    return true;
}

@Override
public Exhibition getItem (int pos){
    return list.get(pos);
}

void resetData() {

    list = originalList;
}

private class ViewHolder {

    ImageView expand,header;

}

@Override
@NonNull
public Filter getFilter() {
    if (valueFilter == null) {
        Log.d("SEARCH1","New filter");
        valueFilter = new ValueFilter();
    }
    return valueFilter;
}

private class ValueFilter extends Filter {
    @Override
    protected FilterResults performFiltering(CharSequence constraint) {

        FilterResults results = new FilterResults();
        if(constraint == null || constraint.length() == 0){
            results.values = originalList;
            results.count = originalList.size();
        }
        else {

            List<Exhibition> nExhList = new ArrayList<>();

            for(Exhibition e : list){
                Log.d("NAMEE",e.name + " " + constraint.toString());
                if (e.getName().toUpperCase().contains(constraint.toString().toUpperCase()) || e.getCity().toUpperCase().contains(constraint.toString().toUpperCase())
                        ||e.getMuseum().toUpperCase().contains(constraint.toString().toUpperCase()) || e.getLongDescription().toUpperCase().contains(constraint.toString().toUpperCase())
                        || e.getDescription().toUpperCase().contains(constraint.toString().toUpperCase()) || e.getCategory().toUpperCase().contains(constraint.toString().toUpperCase())){
                    nExhList.add(e);
                }
            }
            results.values= nExhList;
            results.count=nExhList.size();
        }
        return results;
    }

    @Override
    protected void publishResults(CharSequence constraint,
                                  FilterResults results) {
        if(results.count==0){
            notifyDataSetInvalidated();
        }
        else{
            list = (ArrayList<Exhibition>)results.values;
            notifyDataSetChanged();
        }
    }
}

The first ImageView is a Bitmap stored in a Exhibition variable. The second one changes the visibility of a text to get a expandable-like effect (because for now I can't convert the ListView to a ExpandableListView). I tried different things like the cache, an AsyncTask, removing the custom click listener, put everything in ViewHolder but the scrolling is full of microlags. Is there something wrong in the adapter that I don't get it?

Answer 1

To make your list smooth, you can try this following options,

1. Instead of using your own way of Bitmap caching, you can try to use the popular library like Glide, Picasso or some other opensource
2. Try to avoid doing the time-consuming operation in getView, ex - Date conversion you can move to object level when you build the model object, It will be one time per object.
3. You can try out Recyclerview instead of ListView

Answer 2

There are a couple of things you can do to improve performance.

Figure out exactly what is slow

Learn about profiling which can tell you which functions are the ones that are being called the most and/or that take the longest to complete. This way you can decide where to invest time fixing or changing code.

See https://developer.android.com/studio/profile/android-profiler and https://developer.android.com/studio/profile/

ViewHolder pattern

You're misusing the ViewHolder pattern. In your code you have a single instance of ViewHolder in the adapter's viewHolder field. You then use this field inside the getView() function like a regular local variable.

You then call row.findViewById() multiple times, even if the convertView wasn't null. The findViewById() calls are slow, and the advantage of the view holder is that you only have to call it once per view after expansion (in the convertView==null branch of the if).

Instead you should have 1 view holder per row-view. Note that you're not creating a new ViewHolder to assign with setTag(), but you're reusing the same one. Then instead of the variables such as descr, ttl, city, should be fields of the ViewHolder and thus quick to reference.

Creating unnecessary objects

Memory allocation is also slow.

You're also creating objects each time getView() gets called that you can instead create once total and just reuse.

One such example is the SimpleDateFormat that could be created once in the adapter constructor and simply used to produce the text.

Look into how you can avoid creating so many String objects as well. Formatting with a string buffer or something similar. You don't show the source code for the Exhibition class, so it's not clear why there is a need to create a Date object with the result of calling getStart() and getEnd().

If the 'start' and 'end' fields of the Exhibition objects are never used as longs, consider turning them into immutable Dates during the JSON parsing instead of every time they are used.

Potential slow calls in UI thread

The source code for the Exhibition class is not shown, so we can't tell what the Exhitition.getHeader() function does. If there is bitmap download and/or decoding, moving that to a background thread (and updating after the bitmap is ready) will improve the ListViews scrolling performance.

Unnecessary Calls

There are calls that are being performed even if not needed. For example the assignment of the On Click listener at the end of getView(). You can get away with setting it only once when you do the inflation (when convertView is null) since all the rows use the same listener.

Avoid filling the memory

You mentioned that each Exhibition object has a Bitmap field that is set when the JSON is parsed. This means that all the bitmaps are in memory all the time. This means that in this case the LRU cache is not necessary, since there's always a strong reference to the bitmaps.

It also means that as the number of items in the list increases the memory needed grows. As more memory is used then garbage collection (GC) needs to happens more often, and GC is slow and can cause stutter or freezes. Profiling can tell you if the freezes you're experiencing are due to GC.

The bitmap cache would be useful if there is only a few bitmaps in memory at a time, the ones needed for the items that are currently visible in the list, and a few more. If the needed bitmap is not found in cache then it should be loaded from disk or downloaded from the network.

P.S.

Keep in mind that you have a public setOnCustomClickListener() function that only assigns the reference to the field. If you call that with a new listener your current code will use the old listener on all the rows that weren't refreshed and updated with the new reference.