A lot of things happen during startup before we even start decompressing the libraries. These things take quite some time (several hundreds of milleseconds). It would be beneficial (especially on multi-core systems) to start decompressing the libraries at the earliest time. However, actually initializing Gecko requires that some things are up, otherwise crashes occur when Gecko calls back in Java through JNI.

This starts decompression just after loading libmozglue, but actual Gecko initialization is not moved. From my testing, it doesn't seem to be a loss on single-core devices, but is a clear win on dual-core. If it appears to be a loss on single-core, this could be made conditional to the number of cores (Runtime.availableProcessors may return 1 on multi-core devices, so this would require a native implementation using sysconf(_SC_NPROCESSORS_CONF)) For single-core devices, I've had better startup times by having library decompression and gecko initialization done synchronously, but that blocks UI during that time, so I just dropped that.

Refreshed after bug 760165

Fixed typo.

I'm concerned with how this affects how long it takes us to display about:home. I'd want some timing to to show it doesn't affect it before landing.

This addresses your concern about about:home. Early loading is only triggered when there is an url given to the intent.

>If it appears to be a loss on single-core, this could be made conditional to the >number of cores (Runtime.availableProcessors may return 1 on multi-core devices, >so this would require a native implementation using sysconf(_SC_NPROCESSORS_CONF)) If you you do this, you need to check if bionic libc has the correct implementation. At least in Android 2.3.x era it would get it wrong. See https://bugzilla.mozilla.org/show_bug.cgi?id=663970 Also related see the changelog of NDK 7c.

It would be interesting to run the nighlies before and after this landing to see how startup time is affected on single and dual-core devices, when opening an url and firefox is not started already.

Will, see comment 9

Ran the eideticker startup tests that measure time in seconds to completed web page rendering after launch on a Galaxy Nexus (Android 4.0.4): Results for 2012-06-05 nightly (before landing): 7.716666666666667, 6.666666666666667, 7.083333333333333 Results for 2012-06-06 nightly (after landing): 7.966666666666667, 7.666666666666667, 6.683333333333334 Preliminary interpretation: No perceptible difference. I dug into the data a bit more by watching the videos that eideticker generated, and it looks like we might be seeing the beginnings of a webpage a few hundred milliseconds earlier. The fundamental issue, however, is that it seems like we take longer than the competition to actually get a completely rendered page (bug 765228). That's really what dominates startup time in this case and I suspect it's somewhat variable so it's hard to measure an improvement. Unfortunately I don't currently have a single core device to test with eideticker, so I can't really compare the performance here. Hope this helps!

Based on comment 11 the qawanted keyword can be removed.

Comment on attachment 629731 [details] [diff] [review] Start library decompression earlier Needed to land bug 769269. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): String or UUID changes made by this patch: none

Comment on attachment 629731 [details] [diff] [review] Start library decompression earlier On it's own, I would let this patch ride the trains, but it makes landing bug 769269 easier and it's been baking a fairly long time.

It should be backed out at least from Beta because of the regression: #3 top crasher in 15.0b2.

(In reply to Scoobidiver from comment #16) > It should be backed out at least from Beta because of the regression: #3 top > crasher in 15.0b2. Agreed - comment 11 suggests that this didn't have a major effect on performance, and bug 763166 appears to be a regression caused by this. Let's check in bug 760152 to see how difficult it'll be to land without this patch.

Checked in bug 769269 (not this bug, as the previous comment suggests). Looks like we're good to back this out. glandium - please prepare the backout patch on all branches and nominate for approval.

Not sure if the if !mIsRestoringActivity needs to be larger.

Comment on attachment 647874 [details] [diff] [review] Tentative patch for backout Review of attachment 647874 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine to me. The launch state stuff is all static so it shouldn't need to be wrapped in a mIsRestoringActivity block.

Comment on attachment 647874 [details] [diff] [review] Tentative patch for backout [Approval Request Comment] This is a backout for this bug because we believe it is the reason for a top crash. Kats prefers not to back it out on m-c for now, so that we see if other patches help fix the problem. Testing completed (on m-c, etc.): The patch is untested, but is not completely different from the reverse of the patch that landed. Only a few things changed due to bug 769269. Risk to taking this patch (and alternatives if risky): Low risk, although there could be side effects from the parts that conflicted with bug 769269, but i believe we're early enough in the release process that we would catch these if there are any. String or UUID changes made by this patch: None

Comment on attachment 647874 [details] [diff] [review] Tentative patch for backout [Triage Comment] Since this is untested, we'll start with Aurora and make sure nothing blows up before approving prior to our next beta.

http://hg.mozilla.org/releases/mozilla-aurora/rev/e670dfc55dc8 Not sure what to do with the bug status/flags.

This needs to be backed out of beta too surely? (Bug 763166 is occurring on beta too)

(In reply to Ed Morley [:edmorley] from comment #24) > This needs to be backed out of beta too surely? cf. comment 21 and comment 22, we want to see the backout doesn't break aurora first (since it's not a straight backout, bug 769269 introduced conflicts).

Oops, read comment 21, but missed comment 22. Sorry! :-)

(In reply to Ed Morley [:edmorley] from comment #24) > This needs to be backed out of beta too surely? > (Bug 763166 is occurring on beta too) Looks like this was successful on Aurora, so please do go ahead with backout on Beta.

(In reply to Lukas Blakk [:lsblakk] from comment #27) > Looks like this was successful on Aurora, so please do go ahead with backout > on Beta. What did you use to determine this was successful? It hasn't been long enough for crash-stats to really give meaningful data, I think.

So I was basing my statement on not having a report back after landing that this had broken Aurora, but I see that we need to get crash stat data and will wait then for this information to be gathered a few more days before we make a call on beta approval.

According to bug 763166 comment 52, it looks like the backout fixed the crashes.

Comment on attachment 647874 [details] [diff] [review] Tentative patch for backout Excellent, let's get this out on beta then.

I'd like the backout to land on 17 as well, seeing as backing out this patch fixed the crashes on 15 and 16, and we're still seeing them on 17.

(In reply to Kartikaya Gupta (:kats) from comment #33) > I'd like the backout to land on 17 as well, seeing as backing out this patch > fixed the crashes on 15 and 16, and we're still seeing them on 17. Go ahead, but it would be good if someone from the mobile team could figure out what's going on.

I have a backout patch on try here: https://tbpl.mozilla.org/?tree=Try&rev=abe9add35a5a If it's green I'll push it to inbound. It's basically the same patch that you pushed to aurora.

The try run was green, so I pushed the backout to inbound as well. https://hg.mozilla.org/integration/mozilla-inbound/rev/56ae921df543

(In reply to Kartikaya Gupta (:kats) from comment #36) > The try run was green, so I pushed the backout to inbound as well. > > https://hg.mozilla.org/integration/mozilla-inbound/rev/56ae921df543 Merge of backout: https://hg.mozilla.org/mozilla-central/rev/56ae921df543