Performance improvement suggestion

[andrew659]

Dear developers,

I am a fan of AnySoftKeyboard, and recently I am writing a static code analysis tool to conduct performance analysis for Android apps. I found several violations of "view holder" patterns in AnySoftKeyboard's code. These violations could affect the ListView scrolling performance.

Currently in AnySoftKeyboard, in some list/array adapters the getView() method works like this

public View getView(int position, View convertView, ViewGroup parent) {
View item = mInflater.inflate(R.layout.listItem, null);
((TextView) item.findViewById(R.id.text)).setText(DATA[position]);
((ImageView) item.findViewById(R.id.icon)).setImageBitmap(position & 1) == 1 ? mIcon1 : mIcon2);

return item;
}

When the users scroll a list view, this method is going to build new views continuously instead of using the recycled "convertView". This wastes a lot of CPU cycles in "view inflation" and RAM in storing view objects. On low end devices, GC will kick in frequently because of RAM pressure. This will reduce the UI smoothness when a list has many items (even a list only has a few items, continuously building new views during scrolling is a waste of computational resource).

Google suggested a faster way for getView(), it works like this:

We define a ViewHolder class with two fields: TextView text and ImageView icon. Then the getView() can be implemented like this:

public View getView(int position, View convertView, ViewGroup parent) {
ViewHolder holder;
if(convertView == null){
//we have no recycled views to use, then build new one
convertView = mInflater.inflate(R.layout.listItem, null);
holder = new ViewHolder();
holder.text = (TextView) convertView.getViewById(R.id.text);
holder.icon = (ImageView) convertView.findViewById(R.id.icon);
convertView.setTag(holder)
} else {
//use the recycled view to improve performance
holder = (ViewHolder) convertView.getTag();
}
holder.text.setText(DATA[position]);
holder.icon.setImageBitmap(position & 1) == 1 ? mIcon1 : mIcon2;

return convertView;
}

I noticed that in the following adapters, the view holder pattern is not correctly implemented:

com/anysoftkeyboard/ui/settings/AddOnListPreference.java
com/anysoftkeyboard/ui/settings/UserDictionaryEditorActivity.java

References:
view holder pattern: http://lucasr.org/2012/04/05/performance-tips-for-androids-listview/
http://developer.android.com/training/improving-layouts/smooth-scrolling.html
http://www.youtube.com/watch?v=wDBM6wVEO70

Looking forward to your response and hope I can help improve AnySoftKeyboard :)

[menny]

thanks for providing this analysis!

I'll look into that and apply your suggestions.

Thanks!

[menny]

BTW, did you find additional problems?

[menny]

Also, why to use a ViewHolder class, if I can just do this:

            final View row;
            if (convertView == null) {
                // inflate layout
                LayoutInflater inflator = (LayoutInflater) getContext()
                        .getSystemService(Service.LAYOUT_INFLATER_SERVICE);
                    row = inflator.inflate(R.layout.addon_list_item_pref, parent,
                        false);
            } else {
                row = convertView;
            }

Also, in UserDictionaryEditorActivity there is a call to super.getView(position, convertView, parent); which creates or coverts the view for me (hence does not inflate always).

[andrew659]

I think what you are doing is totally correct. The basic idea is we try to avoid view inflation every time. ViewHolder is just one specific pattern suggested by Android team.

FYI: http://developer.android.com/training/improving-layouts/smooth-scrolling.html

[menny]

Thanks.