Remove support for JSMs
Categories
(Core :: XPConnect, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox136 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
(Blocks 1 open bug)
Details
(Keywords: perf-alert)
Attachments
(13 files, 2 obsolete files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
Once all m-c, c-c, and out-of-tree migrate to ESM, we can remove JSMs.
We're planning to finish the m-c/c-c migration before ESR128, and let out-of-tree code migrate in the ESR128 cycle, and remove JSM support after that.
|
Assignee | |
Updated•9 months ago
|
|
|
Assignee | |
Comment 2•9 months ago
|
||
The patch is almost working,
https://treeherder.mozilla.org/jobs?repo=try&revision=0019c3520e498278a96bcc197f8cea9aeebb2bc4&group_state=expanded
except for that there's one test failing
https://treeherder.mozilla.org/jobs?repo=try&revision=0019c3520e498278a96bcc197f8cea9aeebb2bc4&group_state=expanded&selectedTaskRun=Fwdqg4ItTQaNrWJefJgYJw.0
The issue comes from the patch that unsets the JS::TransitiveCompileOptions.deoptimizeModuleGlobalVars option.
For some reason, the following assertion fails after the change.
The assertion doesn't look actually related to the option (the option marks all top-level variables closed over, that results in putting all variables in the module environment object. this difference shouldn't be visible to regular JS execution, except for slight performance regression).
gDownloadLastDir.file = tmpDir;
clearHistory();
is(gDownloadLastDir.file, null, "gDownloadLastDir.file should be null");
The issue is intermittent locally, so it may take some more time to investigate.
|
|
Assignee | |
Comment 3•9 months ago
|
||
|
||
Comment 4•9 months ago
|
||
Comment on attachment 9446068 [details]
Bug 1881888 - Remove XPCOMUtils.defineLazyModuleGetters. r?mccr8!
Revision D233379 was moved to bug 1940741. Setting attachment 9446068 [details] to obsolete.
|
|
Assignee | |
Comment 5•9 months ago
|
||
|
|
Assignee | |
Comment 6•9 months ago
|
||
The SkipCheckForBrokenURLOrZeroSized flag is introduced by bug 1777641
in order to suppress the assertion while trying to read possibly-non-existent
.jsm url before falling back to the .sys.mjs url.
The fallback is removed and the flag can be removed.
|
|
Assignee | |
Comment 7•9 months ago
|
||
|
|
Assignee | |
Comment 8•9 months ago
|
||
|
|
Assignee | |
Comment 9•9 months ago
|
||
|
|
Assignee | |
Comment 10•9 months ago
|
||
|
|
Assignee | |
Comment 11•9 months ago
|
||
|
|
Assignee | |
Comment 12•9 months ago
|
||
|
|
Assignee | |
Comment 13•9 months ago
|
||
|
|
Assignee | |
Comment 14•9 months ago
|
||
|
|
Assignee | |
Comment 15•9 months ago
|
||
|
|
Assignee | |
Comment 16•9 months ago
|
||
|
|
Assignee | |
Comment 17•9 months ago
|
||
|
|
Assignee | |
Comment 18•9 months ago
|
||
|
|
||
Updated•9 months ago
|
|
|
||
Comment 19•9 months ago
|
||
Comment on attachment 9446630 [details]
Bug 1881888 - Part 13: Stop setting JS::TransitiveCompileOptions.deoptimizeModuleGlobalVars. r?jonco!
Revision D233725 was moved to bug 1941472. Setting attachment 9446630 [details] to obsolete.
|
||
Comment 20•9 months ago
|
||
|
||
Comment 21•8 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/c05980728114
https://hg.mozilla.org/mozilla-central/rev/2d16015a602d
https://hg.mozilla.org/mozilla-central/rev/d967e266d38c
https://hg.mozilla.org/mozilla-central/rev/fb54d847ce61
https://hg.mozilla.org/mozilla-central/rev/afd8f6f2321b
https://hg.mozilla.org/mozilla-central/rev/5b7316fc7e81
https://hg.mozilla.org/mozilla-central/rev/2a5e5d81b362
https://hg.mozilla.org/mozilla-central/rev/c16f489a3f80
https://hg.mozilla.org/mozilla-central/rev/e65703b070ff
https://hg.mozilla.org/mozilla-central/rev/e623df981bc9
https://hg.mozilla.org/mozilla-central/rev/b6642abc8157
https://hg.mozilla.org/mozilla-central/rev/ce9c5f1e511b
https://hg.mozilla.org/mozilla-central/rev/2c7092313122
|
||
Comment 22•8 months ago
|
||
(In reply to Pulsebot from comment #20)
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/c05980728114
Part 1: Remove JSM to ESM shim for ChromeUtils.import. r=jonco
https://hg.mozilla.org/integration/autoland/rev/2d16015a602d
Part 2: Remove SkipCheckForBrokenURLOrZeroSized.
r=jonco,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/d967e266d38c
Part 3: Remove JSM to ESM shim for Cu.isModuleLoaded. r=jonco
https://hg.mozilla.org/integration/autoland/rev/fb54d847ce61
Part 4: Remove JSM to ESM shim for Cu.loadedModules. r=jonco
https://hg.mozilla.org/integration/autoland/rev/afd8f6f2321b
Part 5: Remove Cu.{isModuleLoaded,isJSModuleLoaded}. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/5b7316fc7e81
Part 6: Remove Cu.{loadedModules,loadedJSModules}. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/2a5e5d81b362
Part 7: Remove Cu.unload. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/c16f489a3f80
Part 8: Remove Cu.import. r=mccr8,profiler-reviewers,julienw
https://hg.mozilla.org/integration/autoland/rev/e65703b070ff
Part 9: Remove ChromeUtils.defineModuleGetter.
r=Standard8,mccr8,profiler-reviewers,omc-reviewers,home-newtab-reviewers,
julienw,mconley,pdahiya
https://hg.mozilla.org/integration/autoland/rev/e623df981bc9
Part 10: Remove ChromeUtils.import.
r=Standard8,mccr8,profiler-reviewers,omc-reviewers,home-newtab-reviewers,
julienw,mconley,pdahiya
https://hg.mozilla.org/integration/autoland/rev/b6642abc8157
Part 11: Remove nsIMemoryReporter interface from mozJSModuleLoader. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/ce9c5f1e511b
Part 12: Remove ModuleLoaderInfo::Key. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/2c7092313122
Part 13: Remove unused macro. r=jonco
Perfherder has detected a awsy performance change from push 2c7092313122c684fcdeb6fcf451248ee8645616.
Improvements:
| Ratio | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|
| 0.34% | Base Content JS | windows11-64-2009-shippable-qr | fission | 1,545,552.00 -> 1,540,266.00 |
| 0.33% | Base Content JS | linux1804-64-shippable-qr | fission | 1,539,113.00 -> 1,533,964.00 |
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.
If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.
You can run these tests on try with ./mach try perf --alert 43462
For more information on performance sheriffing please see our FAQ.
|
||
Comment 23•8 months ago
|
||
Maybe the memory wins here are enough to offset the regression in bug 1939273?
|
|
Assignee | |
Comment 24•8 months ago
|
||
Sure, but the situation in bug 1939273 seems to be something we want to address in anyway.
(The extra text affected there doesn't sound reasonable to be in the final binary)
To be clear, I'm not going to backout the entire patch stack just for bug 1939273.
Description
•