Add support for ChromeUtils.import to load files with a .mjs extension as
EcmaScript Modules instead of as JSMs. They continue to be loaded in the
current system component global.

NOTE: We use an '.mjs.jsm' extension for now since parts of the browser still
seem to care about the .jsm extension and I get test failures.

To convert a JSM to a ES-Module perform the following steps:

• Rename the file to .mjs.jsm and update references
• Remove EXPORTED_SYMBOLS and use 'export' keyword
• Remove top-level uses of 'this' as it is defined as 'undefined' in ES
  Modules

NOTE: Using 'import' keyword is untested, but ChromeUtils.import can still be
used within ES Modules.

Depends on D52763

Here is a new attempt at prototyping thing. It needs more testing, but it seems to work a lot better than I expected.

Using a '.mjs' extension was giving me some weird failures, so I'm using '.mjs.jsm' for now and pattern matching inside of ChromeUtils::Import. I haven't tested exotic cases with different globals yet. The keyword import wasn't immediately working, but that is largely due to import path resolution rules I need to investigate.

It seems like the '.mjs' failure was just a mistake on my part. I've fixed the file extension back to the simple '.mjs'. I've also updated the sample patch to convert all the trivial actors to get more testing.

One thing to note is there is no startup cache support for any of this yet, but there is active work related to Bug 1308252.

(In reply to Ted Campbell [:tcampbell] from comment #6)

It seems like the '.mjs' failure was just a mistake on my part. I've fixed the file extension back to the simple '.mjs'. I've also updated the sample patch to convert all the trivial actors to get more testing.

One thing to note is there is no startup cache support for any of this yet, but there is active work related to Bug 1308252.

Thanks Ted! That looks great, and the actor patch is impressively small. Are tests green with that? Once we have good enough perf (at least startup cache), I think landing incremental patches like this separately makes a lot of sense and is something we could break down easily - assuming that we could import a jsm from an mjs such that we don't have cyclical import issues making it so we need to do everything at once.

For the the actor patch, did you manually make those EXPORTED_SYMBOLS -> export changes did you use some kind of script? We should probably make an attempt to migrate a large number of jsm's at once at least locally so that we can run talos to confirm that we aren't going to get a series of small / hard to detect perf regressions if we do it in small batches.

Maybe the .mjs problem is related to bug 1589895?

Just to note, we'll need to update ESLint's settings for the new mjs files. This shouldn't be too complicated, mainly making sure that mjs are seen as modules, and applying the rules/environments that we have for jsm files to mjs as well.

I'm quite happy to help out there at an appropriate time.

(In reply to Tom Schuster [:evilpie] from comment #8)

Maybe the .mjs problem is related to bug 1589895?

I doubt it. I think the mjs extension is working fine actually. I just confused myself trying to test things.

(In reply to Brian Grinstead [:bgrins] from comment #7)

For the the actor patch, did you manually make those EXPORTED_SYMBOLS -> export changes did you use some kind of script? We should probably make an attempt to migrate a large number of jsm's at once at least locally so that we can run talos to confirm that we aren't going to get a series of small / hard to detect perf regressions if we do it in small batches.

This is manually converted right now. I believe :kmag has scripts and ideas for mass conversion though.

Basic mochitests are passing but I'll add a few more now. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e33e9f3c71a1905af2100ebdc39815adb200e566

Performance Concerns I'd like to see checked before landing things:

• Is there perf impact accessing properties of returned module (module namespace proxy) vs the previous plain-object?
• Is the perf of accessing global properties such as 'undefined' actually improved?
• Does startup cache work?
• Does preload/prefetch cache work?

Open Questions

• What is best design for defineLazyModuleGetter replacement?
  • Currently uses a getter on the global that gets redefined to a property. This clobbers some JIT metadata today.
  • Might be possible to enhance ModuleNamespaceObject to do the lazy load. This should be compatible with current JIT optimizations for it.

The basic problem with converting a large number of JSMs at once is that we need to decide how to handle lazy getters, and a large subset of JSMs currently use lazy module getters.

That said, I can write scripts that detect the compatible cases and mass convert all of those.

(In reply to Ted Campbell [:tcampbell] from comment #11)

Performance Concerns I'd like to see checked before landing things:
...

• Is the perf of accessing global properties such as 'undefined' actually improved?

Well, in any case, it can hardly be made worse.

(In reply to Ted Campbell [:tcampbell] from comment #11)

Open Questions

• What is best design for defineLazyModuleGetter replacement?
  • Currently uses a getter on the global that gets redefined to a property. This clobbers some JIT metadata today.
  • Might be possible to enhance ModuleNamespaceObject to do the lazy load. This should be compatible with current JIT optimizations for it.

Is it technically feasible to make them work more or less exactly how we currently use them (support synchronous loading of mjs files at arbitrary times)? I know it's not ideal, and we may lose some JIT benefits, but IIUC it won't leave us in a worse state then we are today. And if we could split the project up into phases like:

1. Support the same loading semantics as JSMs.
2. Do a mass-migration (at least partially automated) where we rename jsm->mjs and rewrite exports.
3. Improve (or stop) lazy loading so we can get JIT wins.

We'd be getting nice wins each step of the way. It also would give us a good target before landing (2) to test performance against the baseline.

(In reply to Brian Grinstead [:bgrins] from comment #13)

Is it technically feasible to make them work more or less exactly how we currently use them (support synchronous loading of mjs files at arbitrary times)? I know it's not ideal, and we may lose some JIT benefits, but IIUC it won't leave us in a worse state then we are today. And if we could split the project up into phases like:

The synchronous loading on-demand should be fine. The issue right now is that they access global this which doesn't exist in .mjs modules. What I'm wondering about is if the value returned from ChomeUtils.import('..mjs') could be automatically lazy. It's type is js::ModuleNamespaceObject but we could add a lazy mode there without impacting JIT support.

let lazyFoo = ChromeUtils.import('foo.mjs');

 function doCoolThings()
{
   lazyFoo.CoolAPI(); // Trigger the module load here.
}

(In reply to Ted Campbell [:tcampbell] from comment #14)

(In reply to Brian Grinstead [:bgrins] from comment #13)

Is it technically feasible to make them work more or less exactly how we currently use them (support synchronous loading of mjs files at arbitrary times)? I know it's not ideal, and we may lose some JIT benefits, but IIUC it won't leave us in a worse state then we are today. And if we could split the project up into phases like:

The synchronous loading on-demand should be fine. The issue right now is that they access global this which doesn't exist in .mjs modules. What I'm wondering about is if the value returned from ChomeUtils.import('..mjs') could be automatically lazy. It's type is js::ModuleNamespaceObject but we could add a lazy mode there without impacting JIT support.

let lazyFoo = ChromeUtils.import('foo.mjs');

 function doCoolThings()
{
   lazyFoo.CoolAPI(); // Trigger the module load here.
}

Oh, that sounds really nice. Seems like if we had that we could rewrite JS that currently uses lazy getters to just eagerly import at the top, but get the same behavior / performance as the lazy getters. Maybe this should be opt in to prevent confusion & diversion from the specified module behavior (ChromeUtils.lazyImport('foo.mjs');?). Although I do suspect we'd see perf wins if we changed it globally from modules that could be lazily loaded but aren't currently.

There are implications to the ergonomics, although most of this applies to existing JSM usage already.

// Content ES Module
import { nameA, nameB } from "foo.mjs";

// Non-Module
let { nameA, nameB } = ChromeUtil.import("foo.mjs");

// Wildcard Import
import * as fooMod from "foo.mjs";

// Hypothetical lazy chrome import
let fooMod = ChromeUtil.import("foo.mjs");

// Dynamic import
import("foo.mjs").then(fooMod => { ... })

In the future, if we use 'import' instead of ChromeUtils, we will still need a non-standard way to replicating this lazy behaviour. For example, we could add an 'importLazy' method to the privileged global that returns the on-demand ModuleNamespaceObject instead of a Promise<ModuleNamespaceObject> that standard dynamic import returns.

In terms of script-based rewriting, :k88hudson's work on https://github.com/k88hudson/babel-plugin-jsm-to-esmodules may offer useful food for thought (or even code re-use?).

(In reply to Dan Mosedale (:dmose, :dmosedale) from comment #17)

In terms of script-based rewriting, :k88hudson's work on https://github.com/k88hudson/babel-plugin-jsm-to-esmodules may offer useful food for thought (or even code re-use?).

Looks like that is the inverse of something I played around with: https://github.com/Mossop/babel-plugin-transform-es2015-modules-mozilla

This is an updated version of the main patch that includes support for the StartupCache. It seems to function OK on smoke-tests, but I ran into a crash well into browsing that may be related. It's a bit weird because the crash itself is an ASSERT failure about an HTTP thread trying to schedule an idle task on the Main thread and failing some sanity checks on which threads are allowed to schedule which jobs where - nothing to do with the underlying changes to the actual cache.

Still investigating that, and still need to add support for the preload cache, which is important for performance.

Rebased version of patch that actually changes a few modules to use ES6 instead of JSM behaviour.

Rebased patch that applies to current tip (10059adf0045).

Depends on D106412

Depends on D113520

I have something that is getting close to being finished, quick update on it:

You will be able to do relative imports now, the same pattern that is used in regular ES6 module imports. You can see this in action here: https://phabricator.services.mozilla.com/D113521

A couple of things I noticed while transitioning certdecoder to the new "module" form.

1. our reliance on requirejs will make some migrations quite frustrating. We might want a pattern that can be broadly applied to allow this. I used rollup to create requirejs like bundles in this case. However, since it was exporting for node's es6 modules, we probably want a different setting than what i used.

2. We may want to not use .mjs -- it may make incremental transitions more difficult. At least, this was my experience when moving cert decoder. What might be the case is it ensures that we transition coherent chunks. That will probably mean we want to have good error messages. The other alternative is -- rather than Cu.import, we create a new funtion Cu.importModule.

2a) If we use mjs, any dependencies that we import from a vendor will also have to be renamed to be .mjs. This could potentially be a huge pain. Jar files also make this a bit difficult, so we might want to say that we adopt a consistent way of loading libraries (such as asn1js & co in the example implementation)

2b) that said, mjs is kind of nice because we don't have any other signal that something is a script import

Applied StartupCache/ScriptPreloader cache to ES6 modules loaded by mozJSComponentLoader::ObjectForLocation*
https://treeherder.mozilla.org/jobs?repo=try&revision=1fdf85abc2e6d73a1989b56e44f235f9a73b1b2a

The change is mostly straightforward. it uses the same caching mechanism as what current JSM uses for regular script stencil,
except that it uses different cachePath than regular script (currently it's just informational. no requirement applied yet)

taking over.

import part will be handled in bug 1311728

I've got some patches (which I'll post shortly) that implement a ChromeUtils.importModule method to loads an ES6 module using the module loader framework.

This turned out to be complicated for several reasons and required a few workarounds, so I'll explain these below:

1. ChromeUtils.importModule is synchronous, whereas the module loader is more event-driven/asynchonrous.

The module loader uses promises internally, both DOM promises and mozJSPromise.

The former happens because the JS API to evaluate a module supports top-level await and hence returns a promise. The patches add an option to get an immediate response when evaluating a module rather than waiting for the promise reactions to run. This is necessary so the new method can to throw an exception when an import fails.

The latter is because the module loader uses mozPromise internally to e.g. wait for module imports to be loaded. The patches work around this by allowing a derived module loader to supply its own event target and creating a trival SyncEventTarget that dispatches runnables by executing them immediately.

2. The module loader API doesn't fit well with how mozJSComponentLoader works.

Firstly, the loader base class lets the derived class implement two operations in particular: fetch module source and compile module source. Currently a vector is used to hold source text or bytecode. However mozJSComponentLoader may use a memory mapped file to access the source, which isn't easy to integrate into this model.

Secondly, mozJSComponentLoader can cache loaded scripts. This relies on having cache key and other contextual information available from fetching available until after compilation when the cache is written to. This could be achieved by storing this information in the load context, but the only purpose this serves is to make mozJSComponentLoader to conform to the module loader model. It has no real gain and only makes the code more confusing.

The patches avoid this and implement both fetch and compile in the fetch step, storing the results in the load context. This is not ideal but requires fewer changes to mozJSComponentLoader.

Comments welcome on this approach. In future we should revisit the loader model model and hopefully find a way to generalise it. Since derived loaders end up having quite different implementations I think it could most sense to combine both these steps into a single operation to get a compiled module, i.e. punt everything to the derived class.

3. AutoJSAPI steals and reports exceptions, but you don't always want that.

The module loader uses AutoJSAPI and AutoEntryScript internally, but these automatically 'report' errors which removes the from the context. We don't want that here so the patches add a way to evaluate a module in a given context.

4. mozJSComponentLoader is not cycle collected and is destroyed after the cycle collector during shutdown.

One final nit. If anything reacable from mozJSComponentLoader (e.g. the new module loader) participates in a cycle then it won't get cleaned up and will leak. This required adding a method to the module loader to manually break any cycles. This is called before the cycle collector shuts down.

For the module loader, since it was made with the Worker/DOM first, I think we can change it however you see fit in order to make this work. Maybe that means taking stuff out, or possibly overriding the functions in question. Since it looks like StartOrRestartModule is the issue.. looking forward to seeing the patches.

It's a bit annoying that we have to convert between the module record object
and module script like this. I filed bug 1766810 to change the APIs but it's a
resonable amount of work at this point.

This adds JS APIs to get the module object for a module script and get a module
namespace object. The former required some refactoring of JSScript::module
(which could return null) into isModule() and module().

As explained in the bug, this adds the option to throw errors immediately
rather than by rejecting a promise. The promise is always settled at this point
since we don't allow top-level await.

Depends on D145554

Depends on D145562

Since mozJSComponentLoader is destroyed after the cycle collector, we need to
break cycles manually before the final cycle collection otherwise we get a
memory leak. Unload() is called at an appropriate earlier point in shutdown.

Depends on D145566

Depends on D145567

This adds the parameter to the module loaders evaluation method. I also
rewrote the comments a bit to make this section clearer based on my
understanding of how this works.

This is called ComponentModuleLoader rather than ModuleLoader as memory leak
detection doesn't like it if we have more than one class with the same name,
even if they're in different namespaces. Other name suggestions welcome.

As explained in the bug, we need to dispatch without using the event loop so we
can complete the import synchronously.

This will be used to hold compilation results before they are passed to the base class.

This adds a flag to ComponentLoaderInfo to say whether it represents a module
and calls the appriate JS APIs based on this.

This is the main patch that finally implements the import.

The derived module loader uses a list of pending load requests to drive the system.

Depends on D145567

https://hg.mozilla.org/mozilla-central/rev/92d99a38d1c5
https://hg.mozilla.org/mozilla-central/rev/27ad92991bf6
https://hg.mozilla.org/mozilla-central/rev/d45158394767
https://hg.mozilla.org/mozilla-central/rev/83f5315e9774
https://hg.mozilla.org/mozilla-central/rev/5e1e77e463a1
https://hg.mozilla.org/mozilla-central/rev/4e3fbb8e9502
https://hg.mozilla.org/mozilla-central/rev/e8b8b8c6454c
https://hg.mozilla.org/mozilla-central/rev/d0348e9694a3
https://hg.mozilla.org/mozilla-central/rev/aa07b7d5d51a
https://hg.mozilla.org/mozilla-central/rev/844364dcbd75
https://hg.mozilla.org/mozilla-central/rev/c7d5d9884fc4
https://hg.mozilla.org/mozilla-central/rev/1a76019fc184
https://hg.mozilla.org/mozilla-central/rev/0684b7a01ac6
https://hg.mozilla.org/mozilla-central/rev/217b34687aa2

Comment on attachment 9218653 [details]
WIP: Bug 1432901 - (WIP) test implementation of certdecoder in network helper with Cu.import

Revision D113521 was moved to bug 1768816. Setting attachment 9218653 [details] to obsolete.