mroshow / bitcoin-wallet

Automatically exported from code.google.com/p/bitcoin-wallet
1 stars 0 forks source link

Performance improvement suggestion #190

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Dear developers,

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

Currently in bitcoin-wallet, 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:
de/schildbach/wallet/ui/FileAdapter.java
de/schildbach/wallet/ui/PeerListFragment.java
de/schildbach/wallet/ui/TransactionsListAdapter.java
de/schildbach/wallet/ui/WalletAddressesAdapter.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 bitcoin-wallet 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 bitcoin-wallet :)

Original issue reported on code.google.com by yepang...@gmail.com on 26 Jun 2013 at 11:41

GoogleCodeExporter commented 9 years ago
Thanks for your suggesting. I'm well aware of the ViewHolder pattern, but think 
that the suggestions mostly stem from a time where processors were still slow.

Because the pattern adds a lot of unwieldy boilerplate code, I decided to use 
it only as an optimization. As of now, none of the list entry layouts are very 
complicated. Most have only 3 or 4 fields.

Do you have a specific list in mind? Which device are you running on?

Note also that Bitcoin Wallet targets Android 4+ devices. Android 2.3 is only 
supported because its still out there, but I really wish I could get rid of it.

Original comment by andreas....@gmail.com on 26 Jun 2013 at 1:17

GoogleCodeExporter commented 9 years ago
I have two devices. One is an HTC Legend running Gingerbread, the other is 
Samsung Galaxy S3 running JellyBean (4.1.2). Bitcoin Wallet is quite smooth on 
my S3, but a little bit sluggish on Legend (for example, the exchange rates 
view).  

I guess my Legend is too old. Not only Bitcoin Wallet is a bit slow, a few 
other apps like k9 mail is not very smooth, either. But still, I wish to see 
some performance optimization as Bitcoin Wallet is really a good app :)

Original comment by yepang...@gmail.com on 26 Jun 2013 at 2:31

GoogleCodeExporter commented 9 years ago
Given that Android is quite popular in developing countries and a lot of those 
devices still run Gingerbread, making it work well there is quite a nice thing 
to do ...

Original comment by hearn@google.com on 28 Jun 2013 at 12:06

GoogleCodeExporter commented 9 years ago
The HTC legend has Android 2.2, which is not supported by Bitcoin Wallet and 
bitcoinj any more. I suggest moving your Bitcoins away from this device.

Original comment by andreas....@gmail.com on 28 Jun 2013 at 1:53

GoogleCodeExporter commented 9 years ago
We have implemented a number for performance enhancements, but the ViewHolder 
pattern does not gain anything these days. We're caching expensive transaction 
values though.

Original comment by andreas....@gmail.com on 16 Nov 2014 at 7:02