ushahidi / Ushahidi_Android

[Deprecated] Ushahidi Android app For Ushahidi V2.x.x. Working on V3.x.x at
https://github.com/ushahidi/platform-android/
GNU Lesser General Public License v3.0
202 stars 153 forks source link

Performance improvement suggestion #146

Closed andrew659 closed 11 years ago

andrew659 commented 11 years ago

Dear developers,

I am a big fan of Ushahidi, 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 Ushahidi's code. These violations could affect the ListView scrolling performance.

Currently in Ushahidi, in some list adapters the getView() method works likes 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 in the com.ushahidi.android.app.adapters package, the view holder pattern is correctly applied:

CategorySpinnerAdapter.java MenuAdapter.java UserSpinnerAdapter.java

However, in the following adapters, the view holder pattern is not correctly implemented:

CommentAdapter.java ListCommentAdapter.java ListMapAdapter.java ListMapTabletAdapter.java ListNewsAdapter.java ListPhotoAdapter.java ListVideoAdapter.java ReportAdapter.java UploadPhotoAdapter.java

So I am curious :) Looking forward to your response. Thanks.

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

eyedol commented 11 years ago

You are right. Those adapters need refactoring. Thanks for reporting this.