1394580 - GeckoView startup optimizations

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

Small optimizations for GeckoView startup

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1394580 - 1. Only load AsyncPrefs for Fennec;

Move AsyncPrefs initialization to inside browser.js to only load it for
Fennec. Also, delay initialization until later in startup.

Review commit: https://reviewboard.mozilla.org/r/175150/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175150/

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1394580 - 2. Add GeckoViewStartup to consolidate startup tasks;

Add a GeckoViewStartup component to consolidate tasks performed during
GeckoView startup, such as adding lazy observers and event listeners.
This enables us to not load individual GeckoView modules such as
GeckoViewPermission and GeckoViewPrompt until they are actually needed.

Review commit: https://reviewboard.mozilla.org/r/175152/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175152/

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1394580 - 3. Lazy-load AndroidLog.jsm;

Lazily load AndroidLog.jsm since we only need it for debug logging, and
logging is normally turned off in GeckoView code.

Review commit: https://reviewboard.mozilla.org/r/175154/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175154/

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1394580 - 4. Disable MDNS service for GeckoView;

Standalone GeckoView doesn't make use of MDNS AFAIK (i.e. MDNS is only
used by features like casting or FlyWeb), but it takes up startup time,
so we should disable it.

Review commit: https://reviewboard.mozilla.org/r/175156/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175156/

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1394580 - 5. Lazy-load Task.jsm in Messaging.jsm;

We're not lazy-loading Task.jsm in Messaging.jsm right now (despite
declaring it through XPCOMUtils.defineLazyModuleGetter) because we
unnecessarily reference `Task` when Messaging.jsm is loading. This patch
makes us load Task.jsm only when `Task` is really needed.

Review commit: https://reviewboard.mozilla.org/r/175158/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175158/

Comment 6

[Eugen Sawin [:esawin]]

Comment on attachment 8903353 [details]
Bug 1394580 - 1. Only load AsyncPrefs for Fennec;

https://reviewboard.mozilla.org/r/175150/#review180386

Comment 7

[Eugen Sawin [:esawin]]

Comment on attachment 8903357 [details]
Bug 1394580 - 5. Lazy-load Task.jsm in Messaging.jsm;

https://reviewboard.mozilla.org/r/175158/#review180394

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8903354 [details]
Bug 1394580 - 2. Add GeckoViewStartup to consolidate startup tasks;

https://reviewboard.mozilla.org/r/175152/#review180488

::: mobile/android/components/geckoview/GeckoViewStartup.js:71
(Diff revision 1)
> +      (ppmm || mm).forEach(msg => target.addMessageListener(msg, listener));
> +    }
> +  },
> +
> +  addLazyEventListener: function(aOptions) {
> +    let {name, target, events, options} = aOptions;

You can just do:

addLazyEventListener({ name, target, events, options}) {

}

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8903356 [details]
Bug 1394580 - 4. Disable MDNS service for GeckoView;

https://reviewboard.mozilla.org/r/175156/#review180490

I think we want to support casting and presentation API, so I think we need to keep this. We're not using the JS implementation, right? How much time does it need on startup?

Comment 10

[Dylan Roeh (:droeh) (he/him)]

Comment on attachment 8903355 [details]
Bug 1394580 - 3. Lazy-load AndroidLog.jsm;

https://reviewboard.mozilla.org/r/175154/#review180508

LGTM

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #9)
> Comment on attachment 8903356 [details]
> Bug 1394580 - 4. Disable MDNS service for GeckoView;
> 
> https://reviewboard.mozilla.org/r/175156/#review180490
> 
> I think we want to support casting and presentation API, so I think we need
> to keep this. We're not using the JS implementation, right? How much time
> does it need on startup?

I guess I see those more as browser features. Seems like the fallback MDNS code is not loaded. Still it does take around 30ms on startup on a Nexus 4.

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Jim Chen [:jchen] [:darchons] from comment #11)
> (In reply to James Willcox (:snorp) ([e-mail]) from comment #9)
> > Comment on attachment 8903356 [details]
> > Bug 1394580 - 4. Disable MDNS service for GeckoView;
> > 
> > https://reviewboard.mozilla.org/r/175156/#review180490
> > 
> > I think we want to support casting and presentation API, so I think we need
> > to keep this. We're not using the JS implementation, right? How much time
> > does it need on startup?
> 
> I guess I see those more as browser features. Seems like the fallback MDNS
> code is not loaded. Still it does take around 30ms on startup on a Nexus 4.

It's a bit of both, right? A browser probably can't cast a web video without some help from the engine. I guess maybe we can disable it for now and come at it again when we decide we want that.

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8903356 [details]
Bug 1394580 - 4. Disable MDNS service for GeckoView;

https://reviewboard.mozilla.org/r/175156/#review180624

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1394580 - 6. Use WindowEventDispatcher for prompt if available;

Review commit: https://reviewboard.mozilla.org/r/176484/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176484/

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903353 [details]
Bug 1394580 - 1. Only load AsyncPrefs for Fennec;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175150/diff/1-2/

Comment 16

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903354 [details]
Bug 1394580 - 2. Add GeckoViewStartup to consolidate startup tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175152/diff/1-2/

Comment 17

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903355 [details]
Bug 1394580 - 3. Lazy-load AndroidLog.jsm;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175154/diff/1-2/

Comment 18

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903356 [details]
Bug 1394580 - 4. Disable MDNS service for GeckoView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175156/diff/1-2/

Comment 19

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903357 [details]
Bug 1394580 - 5. Lazy-load Task.jsm in Messaging.jsm;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175158/diff/1-2/

Comment 20

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8904683 [details]
Bug 1394580 - 6. Use WindowEventDispatcher for prompt if available;

https://reviewboard.mozilla.org/r/176484/#review181460

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b84fc40ed8c2
1. Only load AsyncPrefs for Fennec; r=esawin
https://hg.mozilla.org/integration/autoland/rev/6c4c8b4e38cf
2. Add GeckoViewStartup to consolidate startup tasks; r=snorp
https://hg.mozilla.org/integration/autoland/rev/995d9ef9af5f
3. Lazy-load AndroidLog.jsm; r=droeh
https://hg.mozilla.org/integration/autoland/rev/a9b1cbbeea9a
4. Disable MDNS service for GeckoView; r=snorp
https://hg.mozilla.org/integration/autoland/rev/1cc1955d6af3
5. Lazy-load Task.jsm in Messaging.jsm; r=esawin
https://hg.mozilla.org/integration/autoland/rev/f069f3c69597
6. Use WindowEventDispatcher for prompt if available; r=jchen

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for eslint failure at mobile/android/modules/geckoview/Messaging.jsm:201: Missing space after *:

https://hg.mozilla.org/integration/autoland/rev/5f721a664bf64fed99184a866b60c24a6afcb3a0
https://hg.mozilla.org/integration/autoland/rev/e2953d11ef63a739d6771a0d710f6e92f5405c1e
https://hg.mozilla.org/integration/autoland/rev/90a3ad9bc252daaabb114bf61f8446a90365450a
https://hg.mozilla.org/integration/autoland/rev/05ab3965f981e73908af5a0b328f3a2d2429dc39
https://hg.mozilla.org/integration/autoland/rev/96b5a734399c5f5b4ba228cb2b48c4c8f8a6ba18
https://hg.mozilla.org/integration/autoland/rev/52b106ba3e95059cba7da9c22b3165753d40b3ae

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f069f3c695977904c4b2dda44a5218efe17d28b5&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=128639207&repo=autoland

> TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/mobile/android/modules/geckoview/Messaging.jsm:201:26 | Missing space after *. (generator-star-spacing)

Comment 23

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903353 [details]
Bug 1394580 - 1. Only load AsyncPrefs for Fennec;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175150/diff/2-3/

Comment 24

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903354 [details]
Bug 1394580 - 2. Add GeckoViewStartup to consolidate startup tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175152/diff/2-3/

Comment 25

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903355 [details]
Bug 1394580 - 3. Lazy-load AndroidLog.jsm;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175154/diff/2-3/

Comment 26

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903356 [details]
Bug 1394580 - 4. Disable MDNS service for GeckoView;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175156/diff/2-3/

Comment 27

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8903357 [details]
Bug 1394580 - 5. Lazy-load Task.jsm in Messaging.jsm;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175158/diff/2-3/

Comment 28

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8904683 [details]
Bug 1394580 - 6. Use WindowEventDispatcher for prompt if available;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176484/diff/1-2/

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71322aab413b
1. Only load AsyncPrefs for Fennec; r=esawin
https://hg.mozilla.org/integration/autoland/rev/0fd2ec57de05
2. Add GeckoViewStartup to consolidate startup tasks; r=snorp
https://hg.mozilla.org/integration/autoland/rev/aeb4ccaf74b3
3. Lazy-load AndroidLog.jsm; r=droeh
https://hg.mozilla.org/integration/autoland/rev/dd645a7a4f81
4. Disable MDNS service for GeckoView; r=snorp
https://hg.mozilla.org/integration/autoland/rev/b3b76444d58d
5. Lazy-load Task.jsm in Messaging.jsm; r=esawin
https://hg.mozilla.org/integration/autoland/rev/be2d41805031
6. Use WindowEventDispatcher for prompt if available; r=jchen

Comment 30

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/71322aab413b
https://hg.mozilla.org/mozilla-central/rev/0fd2ec57de05
https://hg.mozilla.org/mozilla-central/rev/aeb4ccaf74b3
https://hg.mozilla.org/mozilla-central/rev/dd645a7a4f81
https://hg.mozilla.org/mozilla-central/rev/b3b76444d58d
https://hg.mozilla.org/mozilla-central/rev/be2d41805031

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef59984ad7b0
7. Follow-up to unpack fields in function signature; r=me

Comment 32

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ef59984ad7b0