Put invocation to "findGameById" into AsyncTask in GameActivity.java?

[yulin2]

Dear Developers of KeepScore,

I'm a Ph.D. student and I'm doing research on improving performance
for Android apps.

I find that in KeepScore, there is a method
"GameDBHelper.findGameById" which accesses database. In most cases,
you put the invocation of this method into AsyncTask to improve responsiveness.
(For example, in "findGameFromGameSummary", "exportToSpreadsheet",
"saveBackup" methods of "MainActivity" class).

But in GameActivity class, "onCreate" method invokes "createGame" at
line 124, which also eventually invokes "GameDBHelper.findGameById" at line
473. I'm whondering why don't also put this invocation of "findGameById"
into background thread? Is there any specific reason that prevent you
to use AsyncTask here?

Thanks,
Yu

[nolanlawson]

Hey, I see that you have some kind of automated static analysis that's finding these performance goofs (see also nolanlawson/Catlog#42). :) Interesting stuff.

Is there any way you could tweak your system to automatically submit a pull request to fix the code? Might not be possible in a lot of cases, but in many, you could just insert a new anonymous AsyncTask to replace the direct database call. Food for thought.

[nolanlawson]

This might also be useful as part of the Android linter.

[yulin2]

I just start looking at how to infer the places where should use concurrency in Android apps.
For the two cases in Catlog and KeepScore, I found them manually (but finally we want to automate it, it's a good idea to integrate it with lint). I thought there might be some specific reasons that you don't put them into background thread…

Actually, we had a paper about how to refactored sequential code to use AsyncTask
(see https://www.ideals.illinois.edu/handle/2142/49882 if interested). I can send a pull request.

[yulin2]

The refactoring for KeepScore would be tricky. The "game" and "playerScores" field need to be synchronized if putting line 473 into background (or putting entire "createGame" method into background thread), otherwise there will be data race. One way to fix these races is to add "synchronized" to those methods which access "game" or "playerScores" field.

What do you think? Is it worth to do such refactoring? I guess you didn't put line 473 into background thread because it requires lots of synchronization?

p.s., we don't have a system to automatically submit a pull request. The tool is used to transform the code only:)