satrion / omnidroid

Automatically exported from code.google.com/p/omnidroid
Apache License 2.0
0 stars 0 forks source link

Performance improvement suggestion #182

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Dear developers,

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

Currently in Omnidroid, 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 "convert" view. This wastes a lot of 
RAM and GC will kick in frequently. It will reduce the UI smoothness on low end 
devices when the users configure more and more rules in Omnidroid.

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;
}

Violations found these classes:
edu.nyu.cs.omnidroid.app.view.simple.ActivityChooseRootEvent.java
edu.nyu.cs.omnidroid.app.view.simple.ActivityDlgActionInput.java
edu.nyu.cs.omnidroid.app.view.simple.ActivityDlgActions.java
edu.nyu.cs.omnidroid.app.view.simple.ActivityDlgApplications.java
edu.nyu.cs.omnidroid.app.view.simple.ActivityDlgAttributes.java
edu.nyu.cs.omnidroid.app.view.simple.ActivityDlgFilters.java
edu.nyu.cs.omnidroid.app.view.simple.ActivityDlgLogTab.java
edu.nyu.cs.omnidroid.app.view.simple.ActivitySavedRules.java
edu.nyu.cs.omnidroid.app.view.simple.AdapterRule.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

If you wish, I can help upload some patches when I have time :)

Original issue reported on code.google.com by andrew...@gmail.com on 26 Jun 2013 at 7:53