Skip to content

Commit

Permalink
[compiler][be] Stabilize compiler output: sort deps and decls by name (…
Browse files Browse the repository at this point in the history 
…#31362)

All dependencies and declarations of a reactive scope can be reordered
to scope start/end. i.e. generated code does not depend on conditional
short-circuiting logic as dependencies are inferred to have no side
effects.

Sorting these by name helps us get higher signal compilation snapshot
diffs when upgrading the compiler and testing PRs
  • Loading branch information
@mofeiZ
mofeiZ authored Nov 5, 2024
1 parent 792fa06 commit f2f002c
Show file tree
Hide file tree
Showing 154 changed files with 732 additions and 685 deletions.
51 changes: 49 additions & 2 deletions compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
ReactiveInstruction,
ReactiveScope,
ReactiveScopeBlock,
ReactiveScopeDeclaration,
ReactiveScopeDependency,
ReactiveTerminal,
ReactiveValue,
Expand Down Expand Up @@ -572,7 +573,8 @@ function codegenReactiveScope(
const changeExpressions: Array<t.Expression> = [];
const changeExpressionComments: Array<string> = [];
const outputComments: Array<string> = [];
for (const dep of scope.dependencies) {

for (const dep of [...scope.dependencies].sort(compareScopeDependency)) {
const index = cx.nextCacheIndex;
changeExpressionComments.push(printDependencyComment(dep));
const comparison = t.binaryExpression(
Expand Down Expand Up @@ -615,7 +617,10 @@ function codegenReactiveScope(
);
}
let firstOutputIndex: number | null = null;
for (const [, {identifier}] of scope.declarations) {

for (const [, {identifier}] of [...scope.declarations].sort(([, a], [, b]) =>
compareScopeDeclaration(a, b),
)) {
const index = cx.nextCacheIndex;
if (firstOutputIndex === null) {
firstOutputIndex = index;
Expand Down Expand Up @@ -2566,3 +2571,45 @@ function convertIdentifier(identifier: Identifier): t.Identifier {
);
return t.identifier(identifier.name.value);
}

function compareScopeDependency(
a: ReactiveScopeDependency,
b: ReactiveScopeDependency,
): number {
CompilerError.invariant(
a.identifier.name?.kind === 'named' && b.identifier.name?.kind === 'named',
{
reason: '[Codegen] Expected named identifier for dependency',
loc: a.identifier.loc,
},
);
const aName = [
a.identifier.name.value,
...a.path.map(entry => `${entry.optional ? '?' : ''}${entry.property}`),
].join('.');
const bName = [
b.identifier.name.value,
...b.path.map(entry => `${entry.optional ? '?' : ''}${entry.property}`),
].join('.');
if (aName < bName) return -1;
else if (aName > bName) return 1;
else return 0;
}

function compareScopeDeclaration(
a: ReactiveScopeDeclaration,
b: ReactiveScopeDeclaration,
): number {
CompilerError.invariant(
a.identifier.name?.kind === 'named' && b.identifier.name?.kind === 'named',
{
reason: '[Codegen] Expected named identifier for declaration',
loc: a.identifier.loc,
},
);
const aName = a.identifier.name.value;
const bName = b.identifier.name.value;
if (aName < bName) return -1;
else if (aName > bName) return 1;
else return 0;
}
6 changes: 3 additions & 3 deletions .../__tests__/fixtures/compiler/allocating-primitive-as-dep-nested-scope.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import { identity, mutate, setProperty } from "shared-runtime";
function AllocatingPrimitiveAsDepNested(props) {
const $ = _c(5);
let t0;
if ($[0] !== props.b || $[1] !== props.a) {
if ($[0] !== props.a || $[1] !== props.b) {
const x = {};
mutate(x);
const t1 = identity(props.b) + 1;
Expand All @@ -62,8 +62,8 @@ function AllocatingPrimitiveAsDepNested(props) {
const y = t2;
setProperty(x, props.a);
t0 = [x, y];
$[0] = props.b;
$[1] = props.a;
$[0] = props.a;
$[1] = props.b;
$[2] = t0;
} else {
t0 = $[2];
Expand Down
6 changes: 3 additions & 3 deletions ...plugin-react-compiler/src/__tests__/fixtures/compiler/array-at-effect.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function ArrayAtTest(props) {
}
const arr = t1;
let t2;
if ($[4] !== props.y || $[5] !== arr) {
if ($[4] !== arr || $[5] !== props.y) {
let t3;
if ($[7] !== props.y) {
t3 = bar(props.y);
Expand All @@ -51,8 +51,8 @@ function ArrayAtTest(props) {
t3 = $[8];
}
t2 = arr.at(t3);
$[4] = props.y;
$[5] = arr;
$[4] = arr;
$[5] = props.y;
$[6] = t2;
} else {
t2 = $[6];
Expand Down
6 changes: 3 additions & 3 deletions ...eact-compiler/src/__tests__/fixtures/compiler/array-expression-spread.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import { c as _c } from "react/compiler-runtime";
function Component(props) {
const $ = _c(3);
let t0;
if ($[0] !== props.foo || $[1] !== props.bar) {
if ($[0] !== props.bar || $[1] !== props.foo) {
t0 = [0, ...props.foo, null, ...props.bar, "z"];
$[0] = props.foo;
$[1] = props.bar;
$[0] = props.bar;
$[1] = props.foo;
$[2] = t0;
} else {
t0 = $[2];
Expand Down
10 changes: 5 additions & 5 deletions ...in-react-compiler/src/__tests__/fixtures/compiler/array-property-call.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@ export const FIXTURE_ENTRYPOINT = {
import { c as _c } from "react/compiler-runtime";
function Component(props) {
const $ = _c(11);
let t0;
let a;
let t0;
if ($[0] !== props.a || $[1] !== props.b) {
a = [props.a, props.b, "hello"];
t0 = a.push(42);
$[0] = props.a;
$[1] = props.b;
$[2] = t0;
$[3] = a;
$[2] = a;
$[3] = t0;
} else {
t0 = $[2];
a = $[3];
a = $[2];
t0 = $[3];
}
const x = t0;
let t1;
Expand Down
6 changes: 3 additions & 3 deletions ...ompiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ function Component() {
throw new Error("invariant broken");
}
let t1;
if ($[1] !== obj || $[2] !== boxedInner) {
if ($[1] !== boxedInner || $[2] !== obj) {
t1 = <Stringify obj={obj} inner={boxedInner} />;
$[1] = obj;
$[2] = boxedInner;
$[1] = boxedInner;
$[2] = obj;
$[3] = t1;
} else {
t1 = $[3];
Expand Down
10 changes: 5 additions & 5 deletions ...computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,16 @@ import { identity, mutate } from "shared-runtime";
*/
function Component(props) {
const $ = _c(8);
let t0;
let key;
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
key = {};
t0 = (mutate(key), key);
$[0] = t0;
$[1] = key;
$[0] = key;
$[1] = t0;
} else {
t0 = $[0];
key = $[1];
key = $[0];
t0 = $[1];
}
const tmp = t0;
let t1;
Expand Down
6 changes: 3 additions & 3 deletions ..._tests__/fixtures/compiler/capturing-fun-alias-captured-mutate-2-iife.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { mutate } from "shared-runtime";
function component(foo, bar) {
const $ = _c(3);
let x;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
x = { foo };
const y = { bar };

Expand All @@ -41,8 +41,8 @@ function component(foo, bar) {
a.x = b;

mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = x;
} else {
x = $[2];
Expand Down
6 changes: 3 additions & 3 deletions ...src/__tests__/fixtures/compiler/capturing-fun-alias-captured-mutate-2.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { c as _c } from "react/compiler-runtime";
function component(foo, bar) {
const $ = _c(3);
let x;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
x = { foo };
const y = { bar };
const f0 = function () {
Expand All @@ -35,8 +35,8 @@ function component(foo, bar) {

f0();
mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = x;
} else {
x = $[2];
Expand Down
6 changes: 3 additions & 3 deletions ...ts__/fixtures/compiler/capturing-fun-alias-captured-mutate-arr-2-iife.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const { mutate } = require("shared-runtime");
function component(foo, bar) {
const $ = _c(3);
let x;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
x = { foo };
const y = { bar };

Expand All @@ -41,8 +41,8 @@ function component(foo, bar) {
a.x = b;

mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = x;
} else {
x = $[2];
Expand Down
6 changes: 3 additions & 3 deletions ...__tests__/fixtures/compiler/capturing-fun-alias-captured-mutate-arr-2.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { c as _c } from "react/compiler-runtime";
function component(foo, bar) {
const $ = _c(3);
let x;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
x = { foo };
const y = { bar };
const f0 = function () {
Expand All @@ -35,8 +35,8 @@ function component(foo, bar) {

f0();
mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = x;
} else {
x = $[2];
Expand Down
6 changes: 3 additions & 3 deletions ...sts__/fixtures/compiler/capturing-func-alias-captured-mutate-arr-iife.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const { mutate } = require("shared-runtime");
function component(foo, bar) {
const $ = _c(3);
let y;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
const x = { foo };
y = { bar };

Expand All @@ -41,8 +41,8 @@ function component(foo, bar) {
a.x = b;

mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = y;
} else {
y = $[2];
Expand Down
6 changes: 3 additions & 3 deletions .../__tests__/fixtures/compiler/capturing-func-alias-captured-mutate-arr.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { c as _c } from "react/compiler-runtime";
function component(foo, bar) {
const $ = _c(3);
let y;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
const x = { foo };
y = { bar };
const f0 = function () {
Expand All @@ -35,8 +35,8 @@ function component(foo, bar) {

f0();
mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = y;
} else {
y = $[2];
Expand Down
6 changes: 3 additions & 3 deletions ...__tests__/fixtures/compiler/capturing-func-alias-captured-mutate-iife.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const { mutate } = require("shared-runtime");
function component(foo, bar) {
const $ = _c(3);
let y;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
const x = { foo };
y = { bar };

Expand All @@ -41,8 +41,8 @@ function component(foo, bar) {
a.x = b;

mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = y;
} else {
y = $[2];
Expand Down
6 changes: 3 additions & 3 deletions .../src/__tests__/fixtures/compiler/capturing-func-alias-captured-mutate.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { c as _c } from "react/compiler-runtime";
function component(foo, bar) {
const $ = _c(3);
let y;
if ($[0] !== foo || $[1] !== bar) {
if ($[0] !== bar || $[1] !== foo) {
const x = { foo };
y = { bar };
const f0 = function () {
Expand All @@ -35,8 +35,8 @@ function component(foo, bar) {

f0();
mutate(y);
$[0] = foo;
$[1] = bar;
$[0] = bar;
$[1] = foo;
$[2] = y;
} else {
y = $[2];
Expand Down
6 changes: 3 additions & 3 deletions ...r/src/__tests__/fixtures/compiler/capturing-function-member-expr-call.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ function component(t0) {
}
const hide = t2;
let t3;
if ($[4] !== poke || $[5] !== hide) {
if ($[4] !== hide || $[5] !== poke) {
t3 = <Foo poke={poke} hide={hide} />;
$[4] = poke;
$[5] = hide;
$[4] = hide;
$[5] = poke;
$[6] = t3;
} else {
t3 = $[6];
Expand Down
Loading

0 comments on commit f2f002c

Please sign in to comment.