Skip to content

Commit c815cdf

Browse files
committed
compiler: Use types to decide which scopes are eligible for merging
In MergeReactiveScopesThatInvalidateTogether when deciding which scopes were eligible for mergin at all, we looked specifically at the instructions whose lvalue produces the declaration. So if a scope declaration was `t0`, we'd love for the instruction where `t0` was the lvalue and look at the instruction type to decide if it is eligible for merging. Here, we use the inferred type instead (now that the inferred types support the same set of types of instructions we looked at before). This allows us to find more cases where scopes can be merged. ghstack-source-id: ab46a9e Pull Request resolved: #29157
1 parent e8e4231 commit c815cdf

18 files changed

+223
-437
lines changed

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts

Lines changed: 3 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {
1313
Place,
1414
ReactiveBlock,
1515
ReactiveFunction,
16-
ReactiveInstruction,
1716
ReactiveScope,
1817
ReactiveScopeBlock,
1918
ReactiveScopeDependencies,
@@ -519,64 +518,7 @@ function scopeIsEligibleForMerging(scopeBlock: ReactiveScopeBlock): boolean {
519518
*/
520519
return true;
521520
}
522-
const visitor = new DeclarationTypeVisitor(scopeBlock.scope);
523-
visitor.visitScope(scopeBlock, undefined);
524-
return visitor.alwaysInvalidatesOnInputChange;
525-
}
526-
527-
class DeclarationTypeVisitor extends ReactiveFunctionVisitor<void> {
528-
scope: ReactiveScope;
529-
alwaysInvalidatesOnInputChange: boolean = false;
530-
531-
constructor(scope: ReactiveScope) {
532-
super();
533-
this.scope = scope;
534-
}
535-
536-
override visitScope(scopeBlock: ReactiveScopeBlock, state: void): void {
537-
if (scopeBlock.scope.id !== this.scope.id) {
538-
return;
539-
}
540-
this.traverseScope(scopeBlock, state);
541-
}
542-
543-
override visitInstruction(
544-
instruction: ReactiveInstruction,
545-
state: void
546-
): void {
547-
this.traverseInstruction(instruction, state);
548-
if (
549-
instruction.lvalue === null ||
550-
!this.scope.declarations.has(instruction.lvalue.identifier.id)
551-
) {
552-
/*
553-
* no lvalue or this instruction isn't directly constructing a
554-
* scope output value, skip
555-
*/
556-
log(
557-
` skip instruction lvalue=${
558-
instruction.lvalue?.identifier.id
559-
} declaration?=${
560-
instruction.lvalue != null &&
561-
this.scope.declarations.has(instruction.lvalue.identifier.id)
562-
} scope=${printReactiveScopeSummary(this.scope)}`
563-
);
564-
return;
565-
}
566-
switch (instruction.value.kind) {
567-
case "FunctionExpression":
568-
case "ArrayExpression":
569-
case "JsxExpression":
570-
case "JsxFragment":
571-
case "ObjectExpression": {
572-
/*
573-
* These instruction types *always* allocate. If they execute
574-
* they will produce a new value, triggering downstream reactive
575-
* updates
576-
*/
577-
this.alwaysInvalidatesOnInputChange = true;
578-
break;
579-
}
580-
}
581-
}
521+
return [...scopeBlock.scope.declarations].some(([, decl]) =>
522+
isAlwaysInvalidatingType(decl.identifier.type)
523+
);
582524
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allocating-primitive-as-dep-nested-scope.expect.md

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,39 +24,28 @@ import { c as _c } from "react/compiler-runtime"; // bar(props.b) is an allocati
2424
// Correctness:
2525
// - y depends on either bar(props.b) or bar(props.b) + 1
2626
function AllocatingPrimitiveAsDepNested(props) {
27-
const $ = _c(9);
28-
let x;
29-
let y;
27+
const $ = _c(5);
28+
let t0;
3029
if ($[0] !== props.b || $[1] !== props.a) {
31-
x = {};
30+
const x = {};
3231
mutate(x);
33-
const t0 = bar(props.b) + 1;
34-
let t1;
35-
if ($[4] !== t0) {
36-
t1 = foo(t0);
37-
$[4] = t0;
38-
$[5] = t1;
32+
const t1 = bar(props.b) + 1;
33+
let t2;
34+
if ($[3] !== t1) {
35+
t2 = foo(t1);
36+
$[3] = t1;
37+
$[4] = t2;
3938
} else {
40-
t1 = $[5];
39+
t2 = $[4];
4140
}
42-
y = t1;
41+
const y = t2;
4342
mutate(x, props.a);
43+
t0 = [x, y];
4444
$[0] = props.b;
4545
$[1] = props.a;
46-
$[2] = x;
47-
$[3] = y;
48-
} else {
49-
x = $[2];
50-
y = $[3];
51-
}
52-
let t0;
53-
if ($[6] !== x || $[7] !== y) {
54-
t0 = [x, y];
55-
$[6] = x;
56-
$[7] = y;
57-
$[8] = t0;
46+
$[2] = t0;
5847
} else {
59-
t0 = $[8];
48+
t0 = $[2];
6049
}
6150
return t0;
6251
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-access-assignment.expect.md

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,40 +24,29 @@ export const FIXTURE_ENTRYPOINT = {
2424
```javascript
2525
import { c as _c } from "react/compiler-runtime";
2626
function foo(a, b, c) {
27-
const $ = _c(10);
28-
let x;
29-
let z;
27+
const $ = _c(6);
28+
let t0;
3029
if ($[0] !== a || $[1] !== b || $[2] !== c) {
31-
x = [a];
32-
let t0;
33-
if ($[5] !== b) {
34-
t0 = [null, b];
35-
$[5] = b;
36-
$[6] = t0;
30+
const x = [a];
31+
let t1;
32+
if ($[4] !== b) {
33+
t1 = [null, b];
34+
$[4] = b;
35+
$[5] = t1;
3736
} else {
38-
t0 = $[6];
37+
t1 = $[5];
3938
}
40-
const y = t0;
41-
z = [[], [], [c]];
39+
const y = t1;
40+
const z = [[], [], [c]];
4241
x[0] = y[1];
4342
z[0][0] = x[0];
43+
t0 = [x, z];
4444
$[0] = a;
4545
$[1] = b;
4646
$[2] = c;
47-
$[3] = x;
48-
$[4] = z;
49-
} else {
50-
x = $[3];
51-
z = $[4];
52-
}
53-
let t0;
54-
if ($[7] !== x || $[8] !== z) {
55-
t0 = [x, z];
56-
$[7] = x;
57-
$[8] = z;
58-
$[9] = t0;
47+
$[3] = t0;
5948
} else {
60-
t0 = $[9];
49+
t0 = $[3];
6150
}
6251
return t0;
6352
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-emit-make-read-only.expect.md

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,20 @@ import { makeReadOnly } from "react-compiler-runtime";
2323
import { c as _c } from "react/compiler-runtime"; // @enableEmitFreeze true
2424

2525
function MyComponentName(props) {
26-
const $ = _c(5);
27-
let x;
26+
const $ = _c(3);
27+
let y;
2828
if ($[0] !== props.a || $[1] !== props.b) {
29-
x = {};
29+
const x = {};
3030
foo(x, props.a);
3131
foo(x, props.b);
32-
$[0] = props.a;
33-
$[1] = props.b;
34-
$[2] = __DEV__ ? makeReadOnly(x, "MyComponentName") : x;
35-
} else {
36-
x = $[2];
37-
}
38-
let y;
39-
if ($[3] !== x) {
32+
4033
y = [];
4134
y.push(x);
42-
$[3] = x;
43-
$[4] = __DEV__ ? makeReadOnly(y, "MyComponentName") : y;
35+
$[0] = props.a;
36+
$[1] = props.b;
37+
$[2] = __DEV__ ? makeReadOnly(y, "MyComponentName") : y;
4438
} else {
45-
y = $[4];
39+
y = $[2];
4640
}
4741
return y;
4842
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-in-statement.expect.md

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,19 @@ export const FIXTURE_ENTRYPOINT = {
2222
```javascript
2323
import { c as _c } from "react/compiler-runtime";
2424
function Component(props) {
25-
const $ = _c(4);
26-
let items;
25+
const $ = _c(2);
26+
let t0;
2727
if ($[0] !== props) {
28-
items = [];
28+
const items = [];
2929
for (const key in props) {
3030
items.push(<div key={key}>{key}</div>);
3131
}
32-
$[0] = props;
33-
$[1] = items;
34-
} else {
35-
items = $[1];
36-
}
37-
let t0;
38-
if ($[2] !== items) {
32+
3933
t0 = <div>{items}</div>;
40-
$[2] = items;
41-
$[3] = t0;
34+
$[0] = props;
35+
$[1] = t0;
4236
} else {
43-
t0 = $[3];
37+
t0 = $[1];
4438
}
4539
return t0;
4640
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/for-loop-with-value-block-initializer.expect.md

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,27 +33,21 @@ export const FIXTURE_ENTRYPOINT = {
3333
import { c as _c } from "react/compiler-runtime";
3434
const TOTAL = 10;
3535
function Component(props) {
36-
const $ = _c(5);
37-
let items;
36+
const $ = _c(3);
37+
let t0;
3838
if ($[0] !== props.start || $[1] !== props.items) {
39-
items = [];
39+
const items = [];
4040
for (let i = props.start ?? 0; i < props.items.length; i++) {
4141
const item = props.items[i];
4242
items.push(<div key={item.id}>{item.value}</div>);
4343
}
44+
45+
t0 = <div>{items}</div>;
4446
$[0] = props.start;
4547
$[1] = props.items;
46-
$[2] = items;
47-
} else {
48-
items = $[2];
49-
}
50-
let t0;
51-
if ($[3] !== items) {
52-
t0 = <div>{items}</div>;
53-
$[3] = items;
54-
$[4] = t0;
48+
$[2] = t0;
5549
} else {
56-
t0 = $[4];
50+
t0 = $[2];
5751
}
5852
return t0;
5953
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-type-inference-array-push.expect.md

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,33 +31,24 @@ export const FIXTURE_ENTRYPOINT = {
3131
```javascript
3232
import { c as _c } from "react/compiler-runtime";
3333
function Component(props) {
34-
const $ = _c(6);
35-
let x;
36-
let y;
34+
const $ = _c(2);
35+
let t0;
3736
if ($[0] !== props) {
38-
x = {};
37+
const x = {};
38+
let y;
3939
if (props.cond) {
4040
y = [props.value];
4141
} else {
4242
y = [];
4343
}
4444

4545
y.push(x);
46-
$[0] = props;
47-
$[1] = x;
48-
$[2] = y;
49-
} else {
50-
x = $[1];
51-
y = $[2];
52-
}
53-
let t0;
54-
if ($[3] !== x || $[4] !== y) {
46+
5547
t0 = [x, y];
56-
$[3] = x;
57-
$[4] = y;
58-
$[5] = t0;
48+
$[0] = props;
49+
$[1] = t0;
5950
} else {
60-
t0 = $[5];
51+
t0 = $[1];
6152
}
6253
return t0;
6354
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-in-other-reactive-block.expect.md

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -37,41 +37,30 @@ import { arrayPush } from "shared-runtime";
3737
// useCallback-produced values can exist in nested reactive blocks, as long
3838
// as their reactive dependencies are a subset of depslist from source
3939
function useFoo(minWidth, otherProp) {
40-
const $ = _c(11);
40+
const $ = _c(7);
4141
const [width] = useState(1);
42-
let style;
43-
let x;
42+
let t0;
4443
if ($[0] !== width || $[1] !== minWidth || $[2] !== otherProp) {
45-
x = [];
46-
let t0;
47-
if ($[5] !== minWidth || $[6] !== width) {
48-
t0 = () => ({ width: Math.max(minWidth, width) });
49-
$[5] = minWidth;
50-
$[6] = width;
51-
$[7] = t0;
44+
const x = [];
45+
let t1;
46+
if ($[4] !== minWidth || $[5] !== width) {
47+
t1 = () => ({ width: Math.max(minWidth, width) });
48+
$[4] = minWidth;
49+
$[5] = width;
50+
$[6] = t1;
5251
} else {
53-
t0 = $[7];
52+
t1 = $[6];
5453
}
55-
style = t0;
54+
const style = t1;
5655

5756
arrayPush(x, otherProp);
57+
t0 = [style, x];
5858
$[0] = width;
5959
$[1] = minWidth;
6060
$[2] = otherProp;
61-
$[3] = style;
62-
$[4] = x;
63-
} else {
64-
style = $[3];
65-
x = $[4];
66-
}
67-
let t0;
68-
if ($[8] !== style || $[9] !== x) {
69-
t0 = [style, x];
70-
$[8] = style;
71-
$[9] = x;
72-
$[10] = t0;
61+
$[3] = t0;
7362
} else {
74-
t0 = $[10];
63+
t0 = $[3];
7564
}
7665
return t0;
7766
}

0 commit comments

Comments
 (0)