totemkevin / imsdroid

Automatically exported from code.google.com/p/imsdroid
0 stars 0 forks source link

Performance improvement suggestion #457

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Dear developers,

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

Currently in imsdroid, in some list adapters the getView() method works likes 
this

public View getView(int position, View convertView, ViewGroup parent) {
  if(convertView == null) {
    convertView = mInflater.inflate(R.layout.listItem, null);
  }
  ((TextView) convertView.findViewById(R.id.text)).setText(DATA[position]);

  return convertView;
}

When the users scroll a list view, this implementation avoids inflations for 
each view (by using the recycled convertView), which saves CPU cycles and RAM. 
However, the method still invokes findViewById() every time when it is called. 
Android documentation says that findViewById is an expensive call, it 
recursively traverses a view tree to find a view matching the give ID. Google 
developers actually suggested a better way to implement getView(). It works 
like this:

We define a ViewHolder class with the field: TextView text . 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);
    convertView.setTag(holder)
  } else {
    //use the recycled view to improve performance
    holder = (ViewHolder) convertView.getTag(); 
  }
  holder.text.setText(DATA[position]);

  return convertView;
}

This avoids calling findViewById frequently and will improve performance when 
the list contains many items or on low end devices.

We found the violations of view holder pattern in these classes:

org/doubango/imsdroid/Screens/ScreenAVQueue.java
org/doubango/imsdroid/Screens/ScreenChat.java
org/doubango/imsdroid/Screens/ScreenChatQueue.java
org/doubango/imsdroid/Screens/ScreenFileTransferQueue.java
org/doubango/imsdroid/Screens/ScreenHome.java
org/doubango/imsdroid/Screens/ScreenPresence.java
org/doubango/imsdroid/Screens/ScreenSettings.java
org/doubango/imsdroid/Screens/ScreenTabContacts.java
org/doubango/imsdroid/Screens/ScreenTabHistory.java
org/doubango/imsdroid/Screens/ScreenTabMessages.java

you may find more useful information in thees 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

In the last Google IO video, we find that the current implementation of 
getView() in imsdroid is a right way, but not a fast way. The video actually 
provides three ways: a slow way, a right way and a fast way.

Looking forward to your reply and hope I can help improve imsdroid :)

Original issue reported on code.google.com by yepang...@gmail.com on 1 Jul 2013 at 11:31