Closed
Bug 1388221
Opened 7 years ago
Closed 7 years ago
Cache existing module exports when defining lazy module getters
Categories
(Core :: XPConnect, enhancement, P2)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
| Performance Impact | high |
People
(Reporter: kmag, Assigned: kmag)
References
(Blocks 1 open bug)
Details
(Keywords: perf, Whiteboard: [fxperf:p3])
Attachments
(1 file)
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•7 years ago
|
||
Is this as simple as making defineLazyModuleGetter do a Cu.isModuleLoaded check and if it's true call Cu.import directly instead of defineLazyGetter?
|
Assignee | |
Comment 2•7 years ago
|
||
(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.
|
||
Updated•7 years ago
|
Whiteboard: [qf] → [qf:p2]
| Comment hidden (mozreview-request) |
|
|
||
Comment 4•7 years ago
|
||
| mozreview-review | ||
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!
Attachment #8895047 -
Flags: review?(florian) → review+
|
|
Assignee | |
Comment 5•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/eca521df662fffe4d3b6de833a01a5f152a3db6e
Bug 1388221: Optimize defineLazyModuleGetter for already-loaded modules. r=florian
|
||
Comment 6•7 years ago
|
||
|
||
Updated•7 years ago
|
Priority: -- → P2
|
|
||
Comment 7•7 years ago
|
||
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?
Flags: needinfo?(kmaglione+bmo)
|
||
Updated•7 years ago
|
Whiteboard: [qf:p2] → [qf:i60][qf:p1]
|
|
||
Updated•7 years ago
|
Whiteboard: [qf:i60][qf:p1] → [qf:f60][qf:p1]
|
|
||
Updated•7 years ago
|
Whiteboard: [qf:f60][qf:p1] → [qf:f61][qf:p1]
|
||
Updated•7 years ago
|
Whiteboard: [qf:f61][qf:p1] → [qf:f61][qf:p1] [fxperf]
|
||
Updated•7 years ago
|
Whiteboard: [qf:f61][qf:p1] [fxperf] → [qf:f61][qf:p1] [fxperf:p3]
|
|
Assignee | |
Comment 8•7 years ago
|
||
I wound up fixing this in ChromeUtils, so these patches aren't necessary anymore.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(kmaglione+bmo)
Resolution: --- → FIXED
|
||
Updated•3 years ago
|
Performance Impact: --- → P1
Whiteboard: [qf:f61][qf:p1] [fxperf:p3] → [fxperf:p3]
You need to log in
before you can comment on or make changes to this bug.
Description
•