put disk access into background thread?

[yulin2]

Hi, I'm doing research on performance for Android apps. I found some event handlers access disk (read/write files, decode bitmap) from UI thread, but Android docs suggest us to avoid such blocking calls in UI thread. Do they lead to any responsiveness issues?

I tried to refactoring by putting them into background tasks. Looking forward to see your comments.

[willlunniss]

Hi, Yes these requests should not be on the UI thread, I guess they aren't impacting performance through which is why it's been left. Unfortunately your approach with an anonymous async task would not handle activity recreation due to say orientation changes. We could either refine this pr or maybe just use something like Picasso (overkill?) to avoid having to write to the boilerplate code.

Pull requests should also be made against the dev branch.

[nelenkov]

Also you don't really want to introduce threads in utility classes (like FileUtils), the user of such classes is responsible to call them in the proper way (background thread, etc). BTW, stating an anonymous background thread that you have no way of controlling is rarely a a good idea.

[yulin2]

Thanks for the comments. I found you use "DetachableAsyncTask" instead of AsyncTask to handle activity recreation, so I refactored the two cases to use DetachableAsyncTask. But there're two problems:

1. Some DetachableAsyncTasks are not attached/detached (e.g., AppInfoActivity.LoadLinksDb) from activity, does it mean such tasks can't handle activity recreation? while some cases like ExportActivity.LoadExportTask do attach/detach. I guess to handle activity recreation, you have to do attach/detach for every background task?
2. Anonymous/regular async task is used in LoginActivity and MainListAdapter. You don't have to handle activity recreation here?

p.s., for NotificationHandler and FileUtils, I remove thread because the disk operations are already invoked in background threads.

[nelenkov]

For this to work, you need to actually call attach()/detach() in onCreate()/onRetainNonConfigurationInstance(), etc. Check Main.java to see how to use it. If some activities don't do it, is an oversight and will probably crash or leak a task. The whole thing is tricky, but I think the general idea is that if your task is really short you can get away with an anonymous task, because it will most probably be over before you can rotate, etc.

Before making major changes, please discuss first. Using a library that all of this easier might be the right choice. I haven't used Picasso but it might worth looking at, at least for loading app icons,
etc.

(To clarify: changing a couple of activities to use detachable tasks is not a major change. Changing how the whole app handles background tasks is. Also do make you PR to the dev branch.)