Closed
Bug 1259045
Opened 8 years ago
Closed 8 years ago
Loader.jsm is loaded early, before browser-delayed-startup-finished
Categories
(DevTools :: General, defect, P3)
DevTools
General
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
Details
(Whiteboard: [btpp-backlog])
Attachments
(1 file, 2 obsolete files)
3.22 KB,
patch
|
ochameau
:
review+
|
Details | Diff | Splinter Review |
Bug 1248600 ensured that devtools code kicks in only once browser-delayed-startup-finished event fires. It is somewhat true. Main modules are no longer loaded before that. But Loader.jsm and some other modules are loaded before. It is being loaded when devtools/shared/event-emitter.js is loaded as a JSM. This module doesn't really depends on any other module, so there is no need for a full module loader. We can save a few cycles/memory when using this module early in firefox startup
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5f200b714ff
Assignee | ||
Comment 2•8 years ago
|
||
Fixed a deprecated patch to event-emitter from browser/ and ensure not using Loader.jsm when used as a JSM.
Assignee | ||
Comment 3•8 years ago
|
||
Forgot to add the deprecated URL from browser/.
Assignee | ||
Updated•8 years ago
|
Attachment #8733881 -
Attachment is obsolete: true
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=618be81f41d8
Assignee | ||
Updated•8 years ago
|
Attachment #8733884 -
Flags: review?(jryans)
Comment on attachment 8733884 [details] [diff] [review] patch v2 Review of attachment 8733884 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/shared/event-emitter.js @@ +36,5 @@ > > this.EventEmitter = function EventEmitter() {}; > module.exports = EventEmitter; > > +// See comment in JSM module boiterplate when adding a new dependency. Nit: boilerplate
Attachment #8733884 -
Flags: review?(jryans) → review+
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8735791 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Attachment #8733884 -
Attachment is obsolete: true
Assignee | ||
Comment 7•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4ad7e36426e
Assignee | ||
Comment 8•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b90f4ecd50d
Assignee | ||
Comment 9•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e56e76ec40cb0e8f11278a5e61414972b771e9fd Bug 1259045 - Prevent loading Loader.jsm when using event-emitter.js as a JSM. r=jryans
Priority: -- → P3
Whiteboard: [btpp-backlog]
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e56e76ec40cb
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Assignee | ||
Comment 11•8 years ago
|
||
This changeset should have moved some computation from early in firefox startup, to after browser-delayed-startup-finished. Over the last months I landed several patch that better control when the devtools code runs and delay it later and later. I'm interested to see if perf test has been improved or regressed on this changeset. Do you know which test could be impacted? first paint? Where can I see the values on mozilla-central changesets?
Flags: needinfo?(jmaher)
Comment 12•8 years ago
|
||
I would expect to see changes in ts_paint, and possible some of the xperf measurements which detect file i/o during startup. There are 4 measures of ts_paint (opt, opt+e10s, pgo, pgo+e10s): https://treeherder.mozilla.org/perf.html#/graphs?timerange=7776000&series=%5Bmozilla-inbound,8a41738d21b57ca7c1c9ebba50ff2239be4091cc,1%5D&series=%5Bmozilla-inbound,619ab4c1115bc476b1b460a67288d28bdc272579,1%5D&series=%5Bmozilla-inbound,e5ca934c64e64394c7fe46a34fdcbce7b2bed374,1%5D&series=%5Bmozilla-inbound,e0b10e5fbf3a8d117e25d51bed07972bae7552ca,1%5D you can see that opt/pgo pretty much track each other- we really only ship pgo, so lets focus on pgo/pgo+e10s: pgo; https://treeherder.mozilla.org/perf.html#/graphs?timerange=7776000&series=%5Bmozilla-inbound,e5ca934c64e64394c7fe46a34fdcbce7b2bed374,1%5D&series=%5Bmozilla-inbound,e0b10e5fbf3a8d117e25d51bed07972bae7552ca,0%5D&series=%5Bmozilla-inbound,32d0b6f0e941cc3061d2ce9b4f36f035a7b30824,1%5D&series=%5Bmozilla-inbound,99bcbbb0fab2bb20d12e679236dc972a0c04b9e2,1%5D&series=%5Bmozilla-inbound,9a06a23a07b2ab578378120a18ec313d607a6f86,1%5D&zoom=1452008302219.1235,1459742349000,731.0706043310624,1080.651179502771 pgo+e10s: https://treeherder.mozilla.org/perf.html#/graphs?timerange=7776000&series=%5Bmozilla-inbound,e0b10e5fbf3a8d117e25d51bed07972bae7552ca,1%5D&series=%5Bmozilla-inbound,6983cd8da36ea94534fd299763eb952590cbbe7b,1%5D&series=%5Bmozilla-inbound,d7dc89dbe0930aad6b3789616401f924097bd192,1%5D&series=%5Bmozilla-inbound,3c1e5da6d0b28a281195f17268f8c04f5430ca76,1%5D overall, I am not seeing anything significant in the ts_paint test. looking at some of the xperf counters (win7 only): https://treeherder.mozilla.org/perf.html#/graphs?timerange=7776000&series=%5Bmozilla-inbound,a5cb3cabdfc19fcf9e4ce69ebc6e910ecc05a633,1%5D&series=%5Bmozilla-inbound,12945b0b398d773c03a01f2a8eb7006e2be4ca82,1%5D&series=%5Bmozilla-inbound,293ca37c03de1e5ff8c774385f682d22ef458b2a,1%5D I don't see any big change. Quite possibly this is a change which doesn't affect performance.
Flags: needinfo?(jmaher)
Assignee | ||
Comment 13•8 years ago
|
||
Thanks for all the links. That's disappointing. I looked at bug 1248600 landing date and don't see any change either. Bug 1248600 is even more significant. I could easily imagine this bug isn't such a big win, but I would have imagine bug 1248600 to have at least a little shake.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•