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 
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 internally at Meta
  • Loading branch information
@mofeiZ
mofeiZ committed Oct 26, 2024
1 parent d19ba8e commit 31f5696
Show file tree
Hide file tree
Showing 133 changed files with 633 additions and 601 deletions.
36 changes: 34 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 @@ -572,7 +572,22 @@ function codegenReactiveScope(
const changeExpressions: Array<t.Expression> = [];
const changeExpressionComments: Array<string> = [];
const outputComments: Array<string> = [];
for (const dep of scope.dependencies) {
const sortedDeps = [...scope.dependencies].sort((a, b) => {
CompilerError.invariant(
a.identifier.name?.kind === 'named' &&
b.identifier.name?.kind === 'named',
{
reason: '[Codegen] Expected dependency to be named',
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;
});
for (const dep of sortedDeps) {
const index = cx.nextCacheIndex;
changeExpressionComments.push(printDependencyComment(dep));
const comparison = t.binaryExpression(
Expand Down Expand Up @@ -615,7 +630,24 @@ function codegenReactiveScope(
);
}
let firstOutputIndex: number | null = null;
for (const [, {identifier}] of scope.declarations) {
// TODO Map<IdentifierId, ReactiveScopeDeclaration>

const sortedDecls = [...scope.declarations].sort(([, a], [, b]) => {
CompilerError.invariant(
a.identifier.name?.kind === 'named' &&
b.identifier.name?.kind === 'named',
{
reason: '[Codegen] Expected dependency to be named',
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;
});
for (const [, {identifier}] of sortedDecls) {
const index = cx.nextCacheIndex;
if (firstOutputIndex === null) {
firstOutputIndex = index;
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
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
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
12 changes: 6 additions & 6 deletions ...babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function Component(props) {
const items = props.items;
const maxItems = props.maxItems;
let renderedItems;
if ($[0] !== maxItems || $[1] !== items) {
if ($[0] !== items || $[1] !== maxItems) {
renderedItems = [];
const seen = new Set();
const max = Math.max(0, maxItems);
Expand All @@ -62,8 +62,8 @@ function Component(props) {
break;
}
}
$[0] = maxItems;
$[1] = items;
$[0] = items;
$[1] = maxItems;
$[2] = renderedItems;
} else {
renderedItems = $[2];
Expand All @@ -79,15 +79,15 @@ function Component(props) {
t0 = $[4];
}
let t1;
if ($[5] !== t0 || $[6] !== renderedItems) {
if ($[5] !== renderedItems || $[6] !== t0) {
t1 = (
<div>
{t0}
{renderedItems}
</div>
);
$[5] = t0;
$[6] = renderedItems;
$[5] = renderedItems;
$[6] = t0;
$[7] = t1;
} else {
t1 = $[7];
Expand Down
6 changes: 3 additions & 3 deletions ...n-react-compiler/src/__tests__/fixtures/compiler/dependencies-outputs.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,16 @@ function foo(a, b) {
x = $[1];
}
let y;
if ($[2] !== x || $[3] !== b) {
if ($[2] !== b || $[3] !== x) {
y = [];
if (x.length) {
y.push(x);
}
if (b) {
y.push(b);
}
$[2] = x;
$[3] = b;
$[2] = b;
$[3] = x;
$[4] = y;
} else {
y = $[4];
Expand Down
8 changes: 4 additions & 4 deletions ...ct-compiler/src/__tests__/fixtures/compiler/destructure-in-branch-ssa.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ function useFoo(props) {
z = $[4];
}
let t0;
if ($[5] !== x || $[6] !== y || $[7] !== myList) {
if ($[5] !== myList || $[6] !== x || $[7] !== y) {
t0 = { x, y, myList };
$[5] = x;
$[6] = y;
$[7] = myList;
$[5] = myList;
$[6] = x;
$[7] = y;
$[8] = t0;
} else {
t0 = $[8];
Expand Down
6 changes: 3 additions & 3 deletions ...ests__/fixtures/compiler/destructuring-same-property-identifier-names.expect.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ function Component(props) {
}
const sameName = t1;
let t2;
if ($[2] !== sameName || $[3] !== renamed) {
if ($[2] !== renamed || $[3] !== sameName) {
t2 = [sameName, renamed];
$[2] = sameName;
$[3] = renamed;
$[2] = renamed;
$[3] = sameName;
$[4] = t2;
} else {
t2 = $[4];
Expand Down
Loading

0 comments on commit 31f5696

Please sign in to comment.