The idnexes which are added are for those columns which
are used to calculate information such as latest upstream version.
These queries use subqueries which seemed to be adversely
impacted by the lack of indexes.
In total, reduced update time on test device from just over 100 seconds
to just over 60 seconds.
This requires renaming the old app/apk tables to be deleted and
the temp ones to be renamed. This is done in a transaction to
ensure we always have at least `fdroid_app` and `fdroid_apk`.
Well, two transactions, one for renaming the `fdroid_app` table
and one for `fdroid_apk`.
Fix 324 : Out of memory errors while updating repos.
Fixes#324, but in the process makes the updater take a lot longer. My benchmarks tell me that an update which used to take approx 30 seconds on my Nexus 4 now takes about 50-55 seconds. This is because it first inserts the apps into the database (in a temp table) and then subsequently copies that table to the actual table. This means there is a lot more disk access than before.
I'm open for discussion on whether this tradeoff is worth it - however I'll caution that there is always going to be a tradeoff between faster and more memory vs slower and less memory. This is the case with all software, and perhaps more so with memory constrained devices such as phones. Also, as the repo index grows (until perhaps we are able to extract the app descriptions in the future), this will become more of an issue.
I'd also like this to be CR'ed properly before merging, because it changes some important code around the repo updater. It is important because security, and it is also important because it is the main thing that F-Droid needs to do (get a list of apps to show the user).
See merge request !173
Will also appear as indeterminate if:
* The repo being downloaded from doesn't send a Content-Length header.
* While connecting to the HTTP server to begin downloading.
Right now it says "50%" always, will need to think whether to ditch the
percentage completely, or to have the temp app/apk providers emit progress
events some how too.
Although not used by the temp provider, it seemed strange having some of
the code always using the `DBHelper.TABLE_APP` and other code using
`getTableName()` where all of it could have used `getTableName()`.
Also moved commiting of the temp tables to the real tables into the
`RepoPersiter` instead of in `RepoUpdater`.
The repo xml handler now has a different mechanism for returning
data about the parsed xml file. This is done via a callback, rather
than storing the data in member variables. The tests now deal with
this correctly.
The update/delete operations of the TempAp[pk]Provider's didn't
work, so that has now been fixed.
At the start of a repo update, it will create a copy of the apk table.
Throughout the update, it will query the original apk table for info.
All inserts and updates happen to the temp table. After the repo has been
verified as trusted, the original apk table is emptied, and all apks are
copied from the temp table to the real one.
I realise that the work done to query the apk table for info during the update
could happen against the temp table, but it was not neccesary to move all of
the queries that are required for this task to the temp apk provider.
Refactored repo update to stream apks from network -> jar file reader ->
xml parser -> database. No longer build up large lists of app metadata
to save. Saves memory, but is MUCH slower.
Does sig verification properly, but does it at the END of the process
and DOESN'T ROLL BACK on failure.
Quick and dirty benchmarks show an increase in time from ~25 seconds
to ~30 seconds on my Nexus 4 with Android 5.0. This doesn't seem so
bad to me, for the tradeoff that people on low end devices can actually
update now.
Also, as @eighthave pointed out, if we are able to stream the download
directly from the internet, then that time will drop to essentially
the time it takes to download the index.
Instead of passing in `null` each time you don't have a username/password,
this change provides those as meaningful default values in an overloaded
version of the method. This takes care of Java's lack of default argument
support.
This is for an abundance of caution. If the guard condition checks
for the presence of both username _and_ password fields, then a crash
or some sort of force close during the update (after adding username
but before password) will mean that next time the app runs, this
condition will evaluate to false and the password field will never
get added.
As with the previous commit, there is probably not any harm doing this
in the way it was done. However it helps reason about the code if
changes are applied in the order that they were introduced. Especially
because each of them does something depending on the version of the
database at that point. With this change, you always know that at the
point that the function is run, the database version will be 51 (and
hence the structure of the database will be predictable).
This may not have caused any trouble, but the principle behind the old
behaviour is that at the point that that was required, the fdroid_repo
table had that particular structure. There is a small chance that it
_may_ have some unintended consequences when upgrading clients with very
old database versions. Probably not, but may as well leave it as is.
