ESM live bindings 2/n: optional experimentalMode arg on importDefault runtime helper for namespace-shaped return - #1867
Open
vzaidman wants to merge 2 commits into
Open
ESM live bindings 2/n: optional experimentalMode arg on importDefault runtime helper for namespace-shaped return#1867vzaidman wants to merge 2 commits into
vzaidman wants to merge 2 commits into
Conversation
Summary:
# Context - live bindings
Metro's `experimentalImportSupport` transform gives ES module imports snapshot semantics. An imported binding is read once, so a later reassignment in the source module is never observed:
```js
// counter.js
export let count = 0;
export function bump() { count++; }
// consumer.js
import {count, bump} from './counter';
bump();
console.log(count); // 0 under Metro today, 1 per spec
```
This is wrong per spec, and it's observable in two scenarios - mutation of an export (as above), which isn't common, and in cycles (below).
## Example - dependency cycle
Cycles are valid in ESM and the following should work as it reads.
```
// a.js
import { B } from './b.js';
export const A = 1;
export function readB() { return B; }
```
```
// b.js
import { A } from './a.js';
export const B = 2;
export function readA() { return A; }
```
```
// main.js
import { readB } from './a.js';
import { readA } from './b.js';
console.log(readA(), readB()); // ESM: 1 2
```
**But with Metro's snapshotting transform**, `a.js` is still evaluating (requiring `b.js`, before it assigns `exports.A`) when `b.js` snapshots its (empty) exports.
```
// a.js
const { B } = require('./b.js'); // b.js completes, 2 is assigned to B
exports.A = 1;
exports.readB = () => B;
```
```
// b.js
const { A } = require('./a.js'); // a.js in progress, its module.exports is still empty {}
exports.B = 2;
exports.readA = () => A; // snapshotted as `undefined`
```
This is an insidious class of bugs that escapes detection by type checkers.
## Performance
Making imports live means a read has to go back to the source on every access rather than binding a value once, so it costs bytes and potentially time. This stack implements liveness first, and then builds on that with optimisations to bring us back to neutral or better.
Liveness stays behind `unstable_liveBindings`, off by default through this stack.
# This diff
The producer half: makes a module's own exports live, so a reassignment after initialisation is visible to importers.
Babel does this by defining an accessor on `exports` for every exported name. Instead this walks each exported binding's `constantViolations` and mirrors the reassignment back as a plain data-property write:
| source | emitted |
| --- | --- |
| `x = v` | `exports.x = (x = v)` |
| `x += v` | `exports.x = (x += v)` |
| `++x` | `exports.x = ++x` |
| `x++` (value unused) | `exports.x = ++x` |
| `x++` (value used) | `(_x = x++, exports.x = x, _x)` |
Reads stay on the plain-property fast path and the cost lands only where a
reassignment actually happens. Only 7 modules in the Wilde graph ever reassign
an exported binding, so this is close to free in practice.
## Why mirroring rather than accessors
Costs land in different places. Accessors pay per exported *name* at init and make every read a getter call. Mirroring pays per *reassignment* and leaves reads as plain property loads.
Measured on the stable SH compiler with the production invocation, and on the release (opt) VM:
| | mirroring | accessors |
| --- | --- | --- |
| HBC per exported name | 26.29 B | 86.03 B |
| HBC per reassignment | 14.59 B | 0 B |
| read | 7.45 ns | 40.80 ns |
| write | 26.80 ns | 6.00 ns |
Mirroring wins on size while a module averages fewer than **4.09 reassignments per exported name**. Across FB-app graph only **7 modules of 44,621** reassign an exported binding at all, so essentially every module pays the 26 B seed and nothing more.
It also puts the CPU cost on the rare operation. Reads outnumber reassignments heavily, and accessors make every one of them 5.5x more expensive.
## Re-exports use getters, not mirroring
Re-exports (`export {x} from './y'`, `export {default as D} from './y'`, `export * from './y'`) require liveness but cannot use mirroring: the reassignment happens in the source module, not this one, so there is no local binding site to hook onto. Snapshotting at re-export time (`exports.x = require('./y').x`) would freeze the value at first read.
The plugin installs a getter:
```js
Object.defineProperty(exports, 'x', {
enumerable: true,
configurable: true,
get: function () { return require('./y').x; },
});
```
## Correctness tradeoff
The cost is that `exports.x` remains an ordinary, writable data property (incorrect, but not a regression vs Metro's current output).
Because this throws under real ESM, it's a pattern that should not exist in the wild. Flow and TS already error on it.
Reviewed By: huntie
Differential Revision: D111529091
… runtime helper for namespace-shaped return
Summary:
Adds an optional `experimentalMode` argument to the `importDefault` runtime helper, selecting the return shape.
## Usage
Existing, unchanged - helper returns the value of the default export
```js
const x = importDefault(id); // exports.default for ESM, exports for CJS
```
`1` — returns a namespace, not memoised:
```js
const ns = importDefault(id, 1); // exports for ESM, {default: exports} for CJS
ns.default;
```
## Why this is important for liveness
The namespace shape lets a default import bind once and read `.default` per use, which is what the `unstable_liveBindings` emission needs to stay live under inline-requires.
Mode 1 skips memoisation because its CJS wrapper is allocated per call — caching it would pin `.default` to the exports object seen on the first call and hide a later `module.exports = X`.
## Why two modes?
*The primary purpose of supporting both is temporary experimentation* - I'm hoping the live stack proves end-to-end perf neutral while being more correct, and we can drop the 2-arg form.
## Why not a new helper, or separate function (`require.importDefaultNamespace`, etc)
- Adding another helper arg to the module wrapper is a lot of churn and inflated HBC on the production (control) path.
- Switching the behaviour of the existing module helper at runtime is viable, but it'd have to be a check inside `require.js`, with `unstable_liveBindings` passed through as a global via prelude or similar. It works but it's intrusive.
- Exposing this functionality other than by a direct module wrapper arg call muddles any perf or HBC size comparison with non-live baseline - `require.<function>()` is slower and bigger than `<function>()`, and resolving a variable from a higher scope is slower than a local scope. In the proposed design, we keep it *almost* equivalent, bar the extra arg we expect to disappear.
Reviewed By: huntie
Differential Revision: D115566590
meta-cla
Bot
added
the
CLA Signed
Contributor
|
@vzaidman has exported this pull request. If you are a Meta employee, you can view the originating Diff in D115566590. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Adds an optional
experimentalModeargument to theimportDefaultruntime helper, selecting the return shape.Usage
Existing, unchanged - helper returns the value of the default export
1— returns a namespace, not memoised:Why this is important for liveness
The namespace shape lets a default import bind once and read
.defaultper use, which is what theunstable_liveBindingsemission needs to stay live under inline-requires.Mode 1 skips memoisation because its CJS wrapper is allocated per call — caching it would pin
.defaultto the exports object seen on the first call and hide a latermodule.exports = X.Why two modes?
The primary purpose of supporting both is temporary experimentation - I'm hoping the live stack proves end-to-end perf neutral while being more correct, and we can drop the 2-arg form.
Why not a new helper, or separate function (
require.importDefaultNamespace, etc)require.js, withunstable_liveBindingspassed through as a global via prelude or similar. It works but it's intrusive.require.<function>()is slower and bigger than<function>(), and resolving a variable from a higher scope is slower than a local scope. In the proposed design, we keep it almost equivalent, bar the extra arg we expect to disappear.Reviewed By: huntie
Differential Revision: D115566590