1388221 - Cache existing module exports when defining lazy module getters

Status: RESOLVED FIXED

(Core :: XPConnect, enhancement, P2)

Description

[Kris Maglione [:kmag]]

We burn a bunch of cycles at startup just creating and populating temporary export holder objects in defineLazyModuleGetter calls, to say nothing of other Cu.import overhead. On my machine, it looks like caching the temporary exports objects saves about 30ms of startup overhead.

Comment 1

[Florian Quèze [:florian]]

Is this as simple as making defineLazyModuleGetter do a Cu.isModuleLoaded check and if it's true call Cu.import directly instead of defineLazyGetter?

Comment 2

[Kris Maglione [:kmag]]

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> Is this as simple as making defineLazyModuleGetter do a Cu.isModuleLoaded
> check and if it's true call Cu.import directly instead of defineLazyGetter?

Hm. No, that's a separate issue, but it's an interesting idea. Defining and then redefining lazy getters tends to get fairly expensive, so that might speed things up even more.

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1388221: Optimize defineLazyModuleGetter for already-loaded modules.

Creating and populating temporary export objects is fairly expensive. Defining
and then redefining lazy getters is sometimes even more expensive.

Caching the export objects from module imports, and immediately defining
properties from already-imported modules appears to save a considerable amount
of overhead at startup.

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

Comment 4

[Florian Quèze [:florian]]

Comment on attachment 8895047 [details]
Bug 1388221: Optimize defineLazyModuleGetter for already-loaded modules.

https://reviewboard.mozilla.org/r/166178/#review171708

Looks good to me, thanks!

Comment 5

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eca521df662fffe4d3b6de833a01a5f152a3db6e
Bug 1388221: Optimize defineLazyModuleGetter for already-loaded modules. r=florian

Comment 6

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/c7bdafee7135 for https://treeherder.mozilla.org/logviewer.html#?job_id=126194914&repo=mozilla-inbound on OS X.

Comment 7

[Andrew Overholt [:overholt]]

Kris, do you think you'll be able to get back to this some time soon? Should it be qf:p1 or is it more of a qf:p3 thing?

Comment 8

[Kris Maglione [:kmag]]

I wound up fixing this in ChromeUtils, so these patches aren't necessary anymore.