Last Comment Bug 713283 - Profile migration takes forever
: Profile migration takes forever
Status: VERIFIED FIXED
[QA+]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P2 major (vote)
: Firefox 12
Assigned To: Gian-Carlo Pascutto [:gcp]
:
Mentors:
: 713282 719125 (view as bug list)
Depends on: 710331 726379
Blocks: 699199 719125
  Show dependency treegraph
 
Reported: 2011-12-23 12:40 PST by Mike Hommey [:glandium]
Modified: 2016-07-29 14:21 PDT (History)
15 users (show)
adriant.mozilla: in‑moztrap+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
verified
verified
verified
beta+
11+


Attachments
Patch 1. Show a setupscreen during migration (2.88 KB, patch)
2012-01-23 15:38 PST, Gian-Carlo Pascutto [:gcp]
doug.turner: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Patch 2. Limit amount of history imported. Sort per importance. (5.58 KB, patch)
2012-01-23 15:40 PST, Gian-Carlo Pascutto [:gcp]
doug.turner: review+
lucasr.at.mozilla: review+
Details | Diff | Splinter Review
Patch 3. Add infrastructure to update visit counts in one call. (12.36 KB, patch)
2012-01-23 15:41 PST, Gian-Carlo Pascutto [:gcp]
doug.turner: review+
lucasr.at.mozilla: review+
Details | Diff | Splinter Review
Patch 0. SetupScreen from bug 710331 (12.02 KB, patch)
2012-01-24 02:24 PST, Gian-Carlo Pascutto [:gcp]
gpascutto: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Patch 2. Limit amount of history imported. Sort per importance. (8.68 KB, patch)
2012-01-24 07:48 PST, Gian-Carlo Pascutto [:gcp]
lucasr.at.mozilla: review-
Details | Diff | Splinter Review
Patch 3. Add infrastructure to update visit counts in one call. (13.00 KB, patch)
2012-01-24 07:48 PST, Gian-Carlo Pascutto [:gcp]
lucasr.at.mozilla: review+
Details | Diff | Splinter Review
Patch 2. Limit amount of history. Update visit counts. (20.28 KB, patch)
2012-01-24 10:05 PST, Gian-Carlo Pascutto [:gcp]
lucasr.at.mozilla: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Patch 3. Merge favicon import in history import (7.09 KB, patch)
2012-01-24 13:23 PST, Gian-Carlo Pascutto [:gcp]
lucasr.at.mozilla: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2011-12-23 12:40:08 PST
My places.sqlite was filled on aurora 10 (thus xul) through sync with my multi-years old desktop profile, and is being migrated in a process that takes forever (Using try build from bug 712718 because without that, migration crashes)

It's been migrating at a pace of a few items a second for the past 30 minutes, and doesn't look like it's near finished.
Comment 1 Mike Hommey [:glandium] 2011-12-23 12:41:01 PST
Device is Nexus S under ICS.
Comment 2 Mike Hommey [:glandium] 2011-12-23 13:22:42 PST
For what it's worth, the moz_places table has 9435 rows in the places.sqlite that is being imported. It's already been spending 1h15 importing it and is not quite done with it. Even it had finished, that would average to less than 3 rows per second.
Comment 3 Mike Hommey [:glandium] 2011-12-23 13:24:12 PST
sqlite> select count(*) from moz_anno_attributes;
9
sqlite> select count(*) from moz_annos;
6
sqlite> select count(*) from moz_bookmarks;
404
sqlite> select count(*) from moz_bookmarks_roots;
5
sqlite> select count(*) from moz_favicons;
21
sqlite> select count(*) from moz_historyvisits;
15829
sqlite> select count(*) from moz_inputhistory;
13
sqlite> select count(*) from moz_items_annos;
55
sqlite> select count(*) from moz_keywords;
4
sqlite> select count(*) from moz_places;
9435
Comment 4 Mike Hommey [:glandium] 2011-12-23 13:29:50 PST
great, it crashed silently :( and then crashed twice in a row at startup.
Comment 5 Mike Hommey [:glandium] 2011-12-23 13:30:18 PST
(it really had been working for an hour and 15 minutes)
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2011-12-29 12:03:56 PST
It seems to me that a better experience would be to throw up a door hanger offering profile migration and when the user initializes profile migration we throw up a blocking dialog showing progress. That way we can use 100% of CPU on the migration, get it done faster and not make the UI laggy (which would be a very poor first run experience).

Madhava, what do you think?
Comment 7 Gian-Carlo Pascutto [:gcp] 2011-12-29 12:10:12 PST
Fully agree on showing a progress dialog.

I think it also makes sense to limit the import to 300 or 500 rows or something. Do we really want to import 10000 history items and block the application for an HOUR? From the description here, it looks like Android does an fsync (rather, SQLite which is used by Android) or something everytime we use the bookmark/history API.
Comment 8 Mike Hommey [:glandium] 2011-12-29 12:19:50 PST
Even with a progress dialog, we should also deprioritize when not the foreground application, because when going back to the android launcher, it was also making the whole thing laggy.

Another issue with importing all history, is that now my awesomebar is completely irrelevant because it apparently lacks frequency information. That's an unrelated issue I guess, but limiting the number of imports as suggested should take that into consideration.
Comment 9 Gian-Carlo Pascutto [:gcp] 2011-12-29 12:46:00 PST
>Another issue with importing all history, is that now my awesomebar is completely 
>irrelevant because it apparently lacks frequency information. That's an unrelated 
>issue I guess, but limiting the number of imports as suggested should take that 
>into consideration.

That should be a separate bug. I'm not sure how the 2 relate? The limit should be on unique URLs in the history DB, not actual visits, so there's no reason it can't work.

Checking the Android source:

http://hi-android.info/src/android/provider/Browser.java.html
private static final int MAX_HISTORY_COUNT = 250;

Oldest 5 entries are deleted whenever we update history. So importing more than 250 entries make no sense.
Comment 10 Mike Hommey [:glandium] 2011-12-29 13:46:59 PST
My point was that if you import a limited number of history entries, awesomebar may become less awesome as a result because of relevance of the entries that have been imported vs. those that have been skipped.
Comment 11 Mike Hommey [:glandium] 2012-01-11 01:27:19 PST
For what it's worth, Aurora has been importing my profile since I filed this bug. As of today, it's not entirely done. It stops when it is OOM killed, and continues (or who knows, maybe it starts over) when aurora is restarted. It's been TWO WEEKS already, this is getting ridiculous.
Comment 12 Madhava Enros [:madhava] 2012-01-13 10:47:50 PST
My understanding is that this is a lot faster now. The ideal UX would be that a user who updates to the new version, and isn't really aware that anything other than a typical update is happening, would find on launching the new version that his or her bookmarks/history are all just there already.

To that end, I think we should take a moment on first run to to this (again, so long as it can be relatively quick). We can show a splash screen with a "Setting Up Firefox" line as we used to in the XUL version. This should get the idea across that what's going on is a one-time thing. We could show some activity or progress indication - but hopefully this is fast enough that we won't need to?
Comment 13 Gian-Carlo Pascutto [:gcp] 2012-01-18 09:51:56 PST
>That should be a separate bug

Tracked in bug 719125.
Comment 14 Mike Hommey [:glandium] 2012-01-22 23:55:23 PST
For what it's worth, it still takes forever on latest nightly, but at least it doesn't crash anymore.
Comment 15 Gian-Carlo Pascutto [:gcp] 2012-01-23 15:38:29 PST
Created attachment 590901 [details] [diff] [review]
Patch 1. Show a setupscreen during migration

This moves the migration to the foreground thread (blocking everything else) while showing a "Setting up X..." screen. Bug 710331 is a prerequisite (but I'll hold of landing that one until this is r+).

Checked performance impact of the animated spinner: not measurable.
Comment 16 Gian-Carlo Pascutto [:gcp] 2012-01-23 15:40:28 PST
Created attachment 590903 [details] [diff] [review]
Patch 2. Limit amount of history imported. Sort per importance.

Limits the number of history entries to 250 (max that Android DB stores). The entries are sorted by the same algorithm as the (mobile!) awesomebar uses, i.e. product of recency and frequency (frecency).
Comment 17 Gian-Carlo Pascutto [:gcp] 2012-01-23 15:41:39 PST
Created attachment 590906 [details] [diff] [review]
Patch 3. Add infrastructure to update visit counts in one call.

Add infrastructure to set history counts directly. Add VISITS to BrowserDB API.
Comment 18 Doug Turner (:dougt) 2012-01-23 21:03:18 PST
Comment on attachment 590901 [details] [diff] [review]
Patch 1. Show a setupscreen during migration

Review of attachment 590901 [details] [diff] [review]:
-----------------------------------------------------------------

need to review SetupScreen.java.  Could you attach?
Comment 19 Doug Turner (:dougt) 2012-01-23 21:07:10 PST
Comment on attachment 590903 [details] [diff] [review]
Patch 2. Limit amount of history imported. Sort per importance.

Review of attachment 590903 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/ProfileMigrator.java
@@ +78,2 @@
>      */
> +    private static final int MAX_HISTORY_COUNT = 250;

This will change with newer devices/os's.  Kinda fragile.  Any better way?
Comment 20 Doug Turner (:dougt) 2012-01-23 21:09:40 PST
Comment on attachment 590906 [details] [diff] [review]
Patch 3. Add infrastructure to update visit counts in one call.

Review of attachment 590906 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/ProfileMigrator.java
@@ +174,5 @@
>              if (allowUpdate) {
>                  BrowserDB.updateVisitedHistory(mCr, url);
>                  BrowserDB.updateHistoryDate(mCr, url, date);
> +                if (hits > 1) {
> +                    BrowserDB.updateHistoryHits(mCr, url, hits - 1);

why -1?

@@ +275,5 @@
>                      String mime = (String)resultRow[mimeCol];
>                      ByteBuffer dataBuff = (ByteBuffer)resultRow[dataCol];
> +                    // Some GIFs can cause us to lock up completely
> +                    // without exceptions or anything. Not cool.
> +                    if (mime.compareTo("image/gif") != 0) {

any idea why?  Please create a follow up bug and reference it here.  We should probably dig into why gif are blowing chunks... but follow-up material for sure.
Comment 21 Gian-Carlo Pascutto [:gcp] 2012-01-24 02:24:28 PST
Created attachment 591046 [details] [diff] [review]
Patch 0. SetupScreen from bug 710331

Reviewed by mfinkle.
Comment 22 Gian-Carlo Pascutto [:gcp] 2012-01-24 02:25:11 PST
Comment on attachment 590901 [details] [diff] [review]
Patch 1. Show a setupscreen during migration

See patch 0.
Comment 23 Lucas Rocha (:lucasr) 2012-01-24 03:13:22 PST
Comment on attachment 590903 [details] [diff] [review]
Patch 2. Limit amount of history imported. Sort per importance.

Review of attachment 590903 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with the small nit fixed.

::: mobile/android/base/ProfileMigrator.java
@@ +78,2 @@
>      */
> +    private static final int MAX_HISTORY_COUNT = 250;

Doug, this hasn't changed up until ICS. So, We can tweak for future releases if necessary.

@@ +103,5 @@
> +        // see BrowserDB.filterAllSites for this formula
> +        + "MAX(1, (((MAX(history.visit_date)/1000) - ?) / 86400000 + 120)) AS a_recent "
> +        + "FROM (moz_historyvisits AS history JOIN moz_places AS places "
> +        + "ON places.id = history.place_id) WHERE places.hidden <> 1 "
> +        + "GROUP BY a_url ORDER BY a_hits * a_recent DESC LIMIT ?";

Looks like a good approach. We'll probably tweak the algorithm on Fennec's local DB a bit soon. Need to remember to update it here too.

@@ +131,5 @@
>          // Get a list of the last times an URL was accessed
>          protected Map<String, Long> gatherAndroidHistory() {
>              Map<String, Long> history = new HashMap<String, Long>();
>  
> +            Cursor cursor = BrowserDB.getRecentHistory(mCr, MAX_HISTORY_COUNT);

Maybe you should expose this value as a public constant in BrowserDB and use it here? To ensure both the DBs and the migrator are using the same value.
Comment 24 Lucas Rocha (:lucasr) 2012-01-24 03:20:56 PST
Comment on attachment 590906 [details] [diff] [review]
Patch 3. Add infrastructure to update visit counts in one call.

Review of attachment 590906 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with nits fixed.

::: mobile/android/base/ProfileMigrator.java
@@ +275,5 @@
>                      String mime = (String)resultRow[mimeCol];
>                      ByteBuffer dataBuff = (ByteBuffer)resultRow[dataCol];
> +                    // Some GIFs can cause us to lock up completely
> +                    // without exceptions or anything. Not cool.
> +                    if (mime.compareTo("image/gif") != 0) {

I think this is because Android's PixmapFactory only support PNG and JPG. But I'd expect it to throw an exception or something...

::: mobile/android/base/db/AndroidBrowserDB.java
@@ +138,5 @@
> +                              null);
> +
> +            if (cursor.moveToFirst()) {
> +                ContentValues values = new ContentValues();
> +

Remove this empty line.

@@ +139,5 @@
> +
> +            if (cursor.moveToFirst()) {
> +                ContentValues values = new ContentValues();
> +
> +                values.put(Browser.BookmarkColumns.VISITS, cursor.getInt(0) + hits);

It would be good to add a comment here explaining why you're adding the hits instead of overwriting the current value. When the migration happens the current visits value will most likely be 0, right?

::: mobile/android/base/db/BrowserDB.java
@@ +65,5 @@
>          public void updateHistoryTitle(ContentResolver cr, String uri, String title);
>  
>          public void updateHistoryDate(ContentResolver cr, String uri, long date);
>  
> +        public void updateHistoryHits(ContentResolver cr, String uri, int hits);

"Hits" is not used anywhere in the database. Maybe you should rename this method to "updateVisitsForUrl" to make it more clear.

::: mobile/android/base/db/LocalBrowserDB.java
@@ +162,5 @@
> +                              null);
> +
> +            if (cursor.moveToFirst()) {
> +                ContentValues values = new ContentValues();
> +

Remove empty line.
Comment 25 Gian-Carlo Pascutto [:gcp] 2012-01-24 07:48:24 PST
Created attachment 591091 [details] [diff] [review]
Patch 2. Limit amount of history imported. Sort per importance.
Comment 26 Gian-Carlo Pascutto [:gcp] 2012-01-24 07:48:58 PST
Created attachment 591092 [details] [diff] [review]
Patch 3. Add infrastructure to update visit counts in one call.
Comment 27 Lucas Rocha (:lucasr) 2012-01-24 08:03:59 PST
Comment on attachment 591091 [details] [diff] [review]
Patch 2. Limit amount of history imported. Sort per importance.

Review of attachment 591091 [details] [diff] [review]:
-----------------------------------------------------------------

mHistoryLimit doesn't seem to be actually used anywhere. Why is this in the patch? I'm giving r- for now. I'd like to get some background before giving r+.

::: mobile/android/base/ProfileMigrator.java
@@ +79,2 @@
>      */
> +    private static final int MAX_HISTORY_COUNT = 2500;

Where does this number come from? I don't follow.

@@ +100,5 @@
>      private final String bookmarkTitle = "a_title";
>  
> +    /*
> +      The sort criterion here corresponds to the one used for the
> +      Awesomebar results. It's an simplifcation of Frecency.

"simplification"

@@ +136,5 @@
>      }
>  
>      private class PlacesTask implements Runnable {
>          // Get a list of the last times an URL was accessed
>          protected Map<String, Long> gatherAndroidHistory() {

Is this actually "Android" history or it might also be native Fennec history (as in "local DB")?

@@ +163,5 @@
>              cursor.close();
>  
> +            // If we get more history entries back than our known minimum,
> +            // we can increase the amount we try to store.
> +            mHistoryLimit = Math.max(historyEntries, MIN_HISTORY_COUNT);

So, we're possibly storing more than MIN_HISTORY_COUNT entries in local DB? We clamp the entries when it exceeds 250 right now. A bit of background would help.
Comment 28 Lucas Rocha (:lucasr) 2012-01-24 08:08:35 PST
Comment on attachment 591092 [details] [diff] [review]
Patch 3. Add infrastructure to update visit counts in one call.

Review of attachment 591092 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/db/BrowserDB.java
@@ +63,5 @@
>          public void updateVisitedHistory(ContentResolver cr, String uri);
>  
>          public void updateHistoryTitle(ContentResolver cr, String uri, String title);
>  
> +        public void updateHistoryAll(ContentResolver cr, String uri, String title,

What does this "all" mean?
Comment 29 Gian-Carlo Pascutto [:gcp] 2012-01-24 09:24:01 PST
>mHistoryLimit doesn't seem to be actually used anywhere. Why is this in the 
>patch?

:-( I'll have to fold the patches. (It's needed in the next patch)

>Maybe you should expose this value as a public constant in BrowserDB and use it 
>here? To ensure both the DBs and the migrator are using the same value.

The problem is that BrowserDB, as far as I can tell, can only ensure the LocalBrowserDB value, not the AndroidDB one. Android makes the value for its own DB private (gee, thanks guys, why'd you make TRUNCATE_N_OLDEST public then?):

    /* Set a cap on the count of history items in the history/bookmark
       table, to prevent db and layout operations from dragging to a
       crawl.  Revisit this cap when/if db/layout performance
       improvements are made.  Note: this does not affect bookmark
       entries -- if the user wants more bookmarks than the cap, they
       get them. */
    private static final int MAX_HISTORY_COUNT = 250;

The only way to read it out is to browse the source.  Given these constraints, I tried to make the Migrator a bit smarter with a heuristic: it asks for "infinite" history entries (MAX_HISTORY_COUNT=2500) and notes how many it actually gets back. If it's less than MIN_HISTORY_COUNT, that just means history was empty and we should use the minimum. If it's more, it can try to store at least this amount.

However, this won't do much for the case of an empty DB which will be pretty typical. 

So maybe BrowserDB should offer getMaxHistoryCount(), call down to LocalBrowserDB and AndroidDB, and AndroidDB should check what version we're running and return the appropriate value. The heuristic can go then.

>It would be good to add a comment here explaining why you're adding the hits instead of 
>overwriting the current value. When the migration happens the current visits value will 
>most likely be 0, right?
...
>Is this actually "Android" history or it might also be native Fennec history (as in "local 
>DB")?
...
>So, we're possibly storing more than MIN_HISTORY_COUNT entries in local DB? We clamp the 
>entries when it exceeds 250 right now. A bit of background would help.

None of this code knows or cares whether it's interacting with AndroidDB or LocalBrowserDB, or whether the database is empty or not. That AndroidDB isn't necessarily empty is obvious. But even LocalBrowserDB might not be, for example if the user downgraded for a while and then upgrades again.

>What does this "all" mean?

Everything. All fields. Maybe "updateHistoryEntry" is better.
Comment 30 Lucas Rocha (:lucasr) 2012-01-24 09:27:19 PST
> So maybe BrowserDB should offer getMaxHistoryCount(), call down to
> LocalBrowserDB and AndroidDB, and AndroidDB should check what version we're
> running and return the appropriate value. The heuristic can go then.

Yeah, that's probably a better plan.
 
> >What does this "all" mean?
> 
> Everything. All fields. Maybe "updateHistoryEntry" is better.

Yep.
Comment 31 Gian-Carlo Pascutto [:gcp] 2012-01-24 10:05:17 PST
Created attachment 591147 [details] [diff] [review]
Patch 2. Limit amount of history. Update visit counts.

Fold pathces. added getMaxHistoryCount(). Remove heuristic. Fix naming Android->BrowserDB, Hits->Visits, HistoryAll->HistoryEntry.
Comment 32 Gian-Carlo Pascutto [:gcp] 2012-01-24 13:23:54 PST
Created attachment 591246 [details] [diff] [review]
Patch 3. Merge favicon import in history import

Favicon import is currently separate from history import. This is not good:
a) Places is smart in linking favicon URLs back to history/bookmark URLs. Android, not so much.
b) We need a separate history query and Map to see which favicons are actually worth getting/storing.
c) Because of lack of frencency information, we read *all* favicons in the Places DB. This will OOM on very large databases.

With this patch and the previous ones, importing a 50Mb places.sqlite (desktop profile used for years) takes 45 seconds on the Galaxy Tab 10.1.
Comment 33 Lucas Rocha (:lucasr) 2012-01-25 03:41:10 PST
Comment on attachment 591147 [details] [diff] [review]
Patch 2. Limit amount of history. Update visit counts.

Review of attachment 591147 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with nits fixed. Please, test those changes using both Android and Local DB before pushing. Although we're not using AndroidBrowserDB right now, it's important to not let it bitrot.

::: mobile/android/base/db/AndroidBrowserDB.java
@@ +126,5 @@
> +            cursor = cr.query(Browser.BOOKMARKS_URI,
> +                              new String[] { Browser.BookmarkColumns.VISITS },
> +                              Browser.BookmarkColumns.URL + " = ?",
> +                              new String[] { uri },
> +                              null);

nit: add empty line here.

@@ +184,4 @@
>      public void clearHistory(ContentResolver cr) {
>          Browser.clearHistory(cr);
>      }
>  

Don't you have to update AndroidBrowserDB's getRecentHistory() to return VISITS too? I only did that in LocalBrowserDB.

::: mobile/android/base/db/LocalBrowserDB.java
@@ +208,5 @@
> +            cursor = cr.query(appendProfile(History.CONTENT_URI),
> +                              new String[] { History.VISITS },
> +                              History.URL + " = ?",
> +                              new String[] { uri },
> +                              null);

nit: add empty line here.
Comment 34 Lucas Rocha (:lucasr) 2012-01-25 03:50:38 PST
Comment on attachment 591246 [details] [diff] [review]
Patch 3. Merge favicon import in history import

Review of attachment 591246 [details] [diff] [review]:
-----------------------------------------------------------------

Please test this on both with AndroidBrowserDB and LocalBrowserDB before pushing.

::: mobile/android/base/ProfileMigrator.java
@@ +104,5 @@
>      private final String historyUrl    = "a_url";
>      private final String historyTitle  = "a_title";
>      private final String historyDate   = "a_date";
>      private final String historyVisits = "a_visits";
> +    private final String faviconData   = "a_data";

Maybe a_favicon_data for clarity?

@@ +105,5 @@
>      private final String historyTitle  = "a_title";
>      private final String historyDate   = "a_date";
>      private final String historyVisits = "a_visits";
> +    private final String faviconData   = "a_data";
> +    private final String faviconMime   = "a_mime";

Maybe a_favicon_mime for clarity?

@@ +188,5 @@
>                  final int urlCol = db.getColumnIndex(historyUrl);
>                  final int titleCol = db.getColumnIndex(historyTitle);
>                  final int dateCol = db.getColumnIndex(historyDate);
>                  final int visitsCol = db.getColumnIndex(historyVisits);
> +                final int mimeCol = db.getColumnIndex(faviconMime);

faviconMimeCol?

@@ +189,5 @@
>                  final int titleCol = db.getColumnIndex(historyTitle);
>                  final int dateCol = db.getColumnIndex(historyDate);
>                  final int visitsCol = db.getColumnIndex(historyVisits);
> +                final int mimeCol = db.getColumnIndex(faviconMime);
> +                final int dataCol = db.getColumnIndex(faviconData);

faviconDataCol?

@@ +203,5 @@
> +                    String mime = (String)resultRow[mimeCol];
> +                    if (mime != null) {
> +                        // Some GIFs can cause us to lock up completely
> +                        // without exceptions or anything. Not cool.
> +                        if (mime.compareTo("image/gif") != 0) {

Are the images with mime type "image/ico" in the database? Do we support those?
Comment 35 Gian-Carlo Pascutto [:gcp] 2012-01-25 09:05:52 PST
>Are the images with mime type "image/ico" in the database? Do we support those?

Apparently, yes, somewhat. See for example some of the comments here:

http://stackoverflow.com/questions/2690503/can-bitmapfactory-decodefile-handle-ico-windows-icons-files 

It's not documented but seems to be fine. 

The code is set up so that it just tries and catches the exception on failure to decode the image data (EAFP). So it'll handle anything anytime Android can. The fact that .gif (which *should* be supported) doesn't but hangs seems to be another Android bug. So this "blacklist" is necessary to prevent us from going dead.
Comment 36 Gian-Carlo Pascutto [:gcp] 2012-01-25 13:17:32 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/70793ab5cd71
https://hg.mozilla.org/integration/mozilla-inbound/rev/325a180c4177
https://hg.mozilla.org/integration/mozilla-inbound/rev/232d8602bdfa

>Please test this on both with AndroidBrowserDB and LocalBrowserDB before pushing.

It works better, though slower on AndroidBrowserDB. I think it's exposing some bugs in LocalBrowserDB. Time to file more bugs.
Comment 37 Gian-Carlo Pascutto [:gcp] 2012-01-25 14:03:50 PST
For the record, this is the performance of the current code:

I/ProfMigr(20644): Bookmarks migration took 19308 ms
I/ProfMigr(20644): Get old history took 8 ms
I/ProfMigr(20644): History query 9894 ms
I/ProfMigr(20644): DB pushing took 71490 ms
I/ProfMigr(20644): history migration took 81392 ms
I/ProfMigr(20644): Profile migration finished
I/GeckoApp(20644): Profile migration took 100788 ms

100s migration, of those only 10s for quering the entire 50M places DB, but 70s to insert the resulting 250 history+favicon entries.

It's inserting into LocalBrowserDB and AndroidDB that's gruesomely slow.
Comment 38 Gian-Carlo Pascutto [:gcp] 2012-01-25 16:03:46 PST
I patched BrowserProvider.java.in, onOpen() to issue a "PRAGMA synchronous=x"

x=OFF: migration takes 38s
x=NORMAL: takes 93s
x=FULL: takes 100s

For reference, desktop uses WAL, which we also use for (Build.VERSION.SDK_INT >= 11). If WAL fails, it uses FULL. Some other parts such as disk cache use OFF.
Comment 39 Gian-Carlo Pascutto [:gcp] 2012-01-25 16:57:49 PST
synchronous = NORMAL
journal = MEMORY

I/ProfMigr(16652): Bookmarks migration took 6504 ms
I/ProfMigr(16652): Get old history took 7 ms
I/ProfMigr(16652): query took 9209 ms
I/ProfMigr(16652): DB pushing took 22906 ms
I/ProfMigr(16652): history migration took 32123 ms
I/ProfMigr(16652): Profile migration finished
I/GeckoApp(16652): Profile migration took 38712 ms

synchronous = NORMAL
journal = TRUNCATE

I/ProfMigr(18759): Bookmarks migration took 13760 ms
I/ProfMigr(18759): Get old history took 4 ms
I/ProfMigr(18759): query took 12992 ms
I/ProfMigr(18759): DB pushing took 49669 ms
I/ProfMigr(18759): history migration took 62666 ms
I/ProfMigr(18759): Profile migration finished
I/GeckoApp(18759): Profile migration took 76481 ms

The latter might be a viable option.
Comment 41 Gian-Carlo Pascutto [:gcp] 2012-01-26 05:33:00 PST
*** Bug 719125 has been marked as a duplicate of this bug. ***
Comment 42 Gian-Carlo Pascutto [:gcp] 2012-01-26 05:35:23 PST
Comment on attachment 591246 [details] [diff] [review]
Patch 3. Merge favicon import in history import

[Approval Request Comment]
Order of magnitude faster profile migration. Fixes wrong history counts.
Comment 43 Alex Keybl [:akeybl] 2012-01-26 16:09:08 PST
Comment on attachment 591046 [details] [diff] [review]
Patch 0. SetupScreen from bug 710331

[Triage Comment]
Mobile only and a blocker for mobile Beta 11. Approved for Aurora 11.
Comment 45 Tony Chung [:tchung] 2012-01-27 18:08:44 PST
Testing has started, and two bugs found so far.  bug 721784 and bug 721782.  

See https://docs.google.com/spreadsheet/ccc?key=0AobT9pFawQ3DdGktOU1hTGVET0x0eUxKbHYxTDlYMGc#gid=0 for the current test matrix.
Comment 46 Tony Chung [:tchung] 2012-01-30 10:55:56 PST
(In reply to Tony Chung [:tchung] from comment #45)
> Testing has started, and two bugs found so far.  bug 721784 and bug 721782.  
> 
> See
> https://docs.google.com/spreadsheet/
> ccc?key=0AobT9pFawQ3DdGktOU1hTGVET0x0eUxKbHYxTDlYMGc#gid=0 for the current
> test matrix.

bug 721934 is of concern here.  we need more time to focus on Profile Migration testing.
Comment 47 Gian-Carlo Pascutto [:gcp] 2012-01-30 13:29:14 PST
*** Bug 713282 has been marked as a duplicate of this bug. ***
Comment 48 Camelia Urian 2012-02-10 06:32:27 PST
Nightly 13.0a1 (2012-02-10)
Aurora 12.0a2 (2012-02-09)

Verified on both Aurora and Nightly: for 7MB profiles, it takes about 3m40sec for migration and for 2MB profile migration is completed in about 1m10sec.
Comment 49 Aaron Train [:aaronmt] 2012-02-10 06:40:27 PST
3 minutes and 40 seconds for 7MB profiles cant be normal now can it? The above was tested on an Nexus S.
Comment 50 Ioana Chiorean 2012-04-17 05:13:59 PDT
Verification of the bug depends on Bug 726379 - crash while migration after sync
Comment 51 Cristian Nicolae (:xti) 2012-05-23 07:42:57 PDT
Profile migration takes less than 30s on the latest Nightly. Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-23)
Device: Galaxy Nexus
OS: Android 4.0.2
Comment 52 Adrian Tamas (:AdrianT) 2012-09-03 07:10:24 PDT
Covered by existing test case:

https://moztrap.mozilla.org/manage/cases/_detail/5490/ 

which is included in the BFTs run in the Firefox Sync test suite

Note You need to log in before you can comment on or make changes to this bug.