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?
There are a couple of things you can do to improve performance.
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/
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.
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 long
s, consider turning them into immutable Date
s during the JSON parsing instead of every time they are used.
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 ListView
s scrolling performance.
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.
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.