Closed Bug 837816 Opened 7 years ago Closed 7 years ago

GeckoAppShell.loadMozGlue gets called three times on startup


(Firefox for Android :: General, defect)

Not set



Firefox 21


(Reporter: kats, Assigned: kats)



(1 file)

Once from GeckoAppShell.loadSQLiteLibs, once from GeckoAppShell.loadNSSLibs, and once from GeckoApp.onCreate, just for good measure. (Not necessarily in that order)
Also, I just noticed the synchronization blocks in loadNSSLibs and loadSQLiteLibs are pretty bad. The sSQLiteLibsLoaded and sNSSLibsLoaded objects are booleans, and specifically Boolean.TRUE and Boolean.FALSE because of autoboxing. That means any other code accidentally synchronizing on those objects will block these functions (I verified this by inserting this code in GeckoApp.create:

        new Thread() {
            public void run() {
                synchronized (Boolean.FALSE) {
                    try {
                        Thread.sleep(60 * 1000);
                    } catch (Exception e) {

and noticed that it blocked gecko startup for a minute, as expected). Additionally the object being synchronized on is reassigned within the synchronization block, which is also generally bad practice and can defeat the synchronization block in subtle ways on platforms where assignments are not atomic.
Working on a fix.
Assignee: nobody → bugmail.mozilla
Attached patch PatchSplinter Review
This does a bunch of cleanup with the bool/locking to fix the issues I identified above. It also adds a new bool to guard against loading mozglue multiple times unnecessarily.

Try run at - I expect this to show a startup time improvement as well.
Attachment #709908 - Flags: review?(cpeterson)
Comment on attachment 709908 [details] [diff] [review]

Review of attachment 709908 [details] [diff] [review]:

Attachment #709908 - Flags: review?(cpeterson) → review+
(In reply to Kartikaya Gupta ( from comment #3)
> Try run at - I expect
> this to show a startup time improvement as well.

The ts startup times from this try push look a little worse (compared to m-i).
I looked at the recent inbound ts numbers before and after my change. Looks like it got a bit worse on 2.2 and a bit better on 4.0 but on both it's well within the margin of error. I'd call it insigificant.

               | Before my change | After my change
Android 2.2 ts | Samples: 16      | Samples: 21
               | Average: 4163.5  | Average: 4580.86
               | Stddev: 1285.51  | Stddev: 1257.27
               | Max: 5564        | Max: 5708
               | Min: 3506        | Min: 3518
Android 4.0 ts | Samples: 17      | Samples: 21
               | Average: 3881.53 | Average: 3866.29
               | Stddev: 944.467  | Stddev: 846.491
               | Max: 4047        | Max: 4016
               | Min: 3738        | Min: 3703
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
You need to log in before you can comment on or make changes to this bug.