Skip to content

Commit abf0b88

Browse files
committed
[compiler] Improve merging of scopes that invalidate together
We try to merge consecutive reactive scopes that will always invalidate together, but there's one common case that isn't handled. ```js const y = [[x]]; ``` Here we'll create two consecutive scopes for the inner and outer array expressions. Because the input to the second scope is a temporary, they'll merge into one scope. But if we name the inner array, the merging stops: ```js const array = [x]; const y = [array]; ``` This is because the merging logic checks if all the dependencies of the second scope are outputs of the first scope, but doesn't account for renaming due to LoadLocal/StoreLocal. The fix is to track these temporaries.
1 parent bfd53a3 commit abf0b88

File tree

60 files changed

+446
-881
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+446
-881
lines changed

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ class FindLastUsageVisitor extends ReactiveFunctionVisitor<void> {
119119

120120
class Transform extends ReactiveFunctionTransform<ReactiveScopeDependencies | null> {
121121
lastUsage: Map<DeclarationId, InstructionId>;
122+
temporaries: Map<DeclarationId, DeclarationId> = new Map();
122123

123124
constructor(lastUsage: Map<DeclarationId, InstructionId>) {
124125
super();
@@ -215,6 +216,12 @@ class Transform extends ReactiveFunctionTransform<ReactiveScopeDependencies | nu
215216
current.lvalues.add(
216217
instr.instruction.lvalue.identifier.declarationId,
217218
);
219+
if (instr.instruction.value.kind === 'LoadLocal') {
220+
this.temporaries.set(
221+
instr.instruction.lvalue.identifier.declarationId,
222+
instr.instruction.value.place.identifier.declarationId,
223+
);
224+
}
218225
}
219226
break;
220227
}
@@ -236,6 +243,13 @@ class Transform extends ReactiveFunctionTransform<ReactiveScopeDependencies | nu
236243
)) {
237244
current.lvalues.add(lvalue.identifier.declarationId);
238245
}
246+
this.temporaries.set(
247+
instr.instruction.value.lvalue.place.identifier
248+
.declarationId,
249+
this.temporaries.get(
250+
instr.instruction.value.value.identifier.declarationId,
251+
) ?? instr.instruction.value.value.identifier.declarationId,
252+
);
239253
} else {
240254
log(
241255
`Reset scope @${current.block.scope.id} from StoreLocal in [${instr.instruction.id}]`,
@@ -260,7 +274,7 @@ class Transform extends ReactiveFunctionTransform<ReactiveScopeDependencies | nu
260274
case 'scope': {
261275
if (
262276
current !== null &&
263-
canMergeScopes(current.block, instr) &&
277+
canMergeScopes(current.block, instr, this.temporaries) &&
264278
areLValuesLastUsedByScope(
265279
instr.scope,
266280
current.lvalues,
@@ -426,6 +440,7 @@ function areLValuesLastUsedByScope(
426440
function canMergeScopes(
427441
current: ReactiveScopeBlock,
428442
next: ReactiveScopeBlock,
443+
temporaries: Map<DeclarationId, DeclarationId>,
429444
): boolean {
430445
// Don't merge scopes with reassignments
431446
if (
@@ -469,16 +484,22 @@ function canMergeScopes(
469484
Iterable_some(
470485
current.scope.declarations.values(),
471486
decl =>
472-
decl.identifier.declarationId === dep.identifier.declarationId,
487+
decl.identifier.declarationId === dep.identifier.declarationId ||
488+
decl.identifier.declarationId ===
489+
temporaries.get(dep.identifier.declarationId),
473490
),
474491
))
475492
) {
476493
log(` outputs of prev are input to current`);
477494
return true;
478495
}
479496
log(` cannot merge scopes:`);
480-
log(` ${printReactiveScopeSummary(current.scope)}`);
481-
log(` ${printReactiveScopeSummary(next.scope)}`);
497+
log(
498+
` ${printReactiveScopeSummary(current.scope)} ${[...current.scope.declarations.values()].map(decl => decl.identifier.declarationId)}`,
499+
);
500+
log(
501+
` ${printReactiveScopeSummary(next.scope)} ${[...next.scope.dependencies].map(dep => `${dep.identifier.declarationId} ${temporaries.get(dep.identifier.declarationId) ?? dep.identifier.declarationId}`)}`,
502+
);
482503
return false;
483504
}
484505

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-at-closure.expect.md

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ function Component(props) {
1919
```javascript
2020
import { c as _c } from "react/compiler-runtime";
2121
function Component(props) {
22-
const $ = _c(7);
22+
const $ = _c(5);
2323
let t0;
2424
if ($[0] !== props.x) {
2525
t0 = foo(props.x);
@@ -31,26 +31,19 @@ function Component(props) {
3131
const x = t0;
3232
let t1;
3333
if ($[2] !== props || $[3] !== x) {
34-
t1 = function () {
34+
const fn = function () {
3535
const arr = [...bar(props)];
3636
return arr.at(x);
3737
};
38+
39+
t1 = fn();
3840
$[2] = props;
3941
$[3] = x;
4042
$[4] = t1;
4143
} else {
4244
t1 = $[4];
4345
}
44-
const fn = t1;
45-
let t2;
46-
if ($[5] !== fn) {
47-
t2 = fn();
48-
$[5] = fn;
49-
$[6] = t2;
50-
} else {
51-
t2 = $[6];
52-
}
53-
const fnResult = t2;
46+
const fnResult = t1;
5447
return fnResult;
5548
}
5649

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-captures-receiver-noAlias.expect.md

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,34 +23,18 @@ export const FIXTURE_ENTRYPOINT = {
2323
```javascript
2424
import { c as _c } from "react/compiler-runtime";
2525
function Component(props) {
26-
const $ = _c(6);
26+
const $ = _c(2);
2727
let t0;
2828
if ($[0] !== props.a) {
29-
t0 = { a: props.a };
29+
const item = { a: props.a };
30+
const items = [item];
31+
t0 = items.map(_temp);
3032
$[0] = props.a;
3133
$[1] = t0;
3234
} else {
3335
t0 = $[1];
3436
}
35-
const item = t0;
36-
let t1;
37-
if ($[2] !== item) {
38-
t1 = [item];
39-
$[2] = item;
40-
$[3] = t1;
41-
} else {
42-
t1 = $[3];
43-
}
44-
const items = t1;
45-
let t2;
46-
if ($[4] !== items) {
47-
t2 = items.map(_temp);
48-
$[4] = items;
49-
$[5] = t2;
50-
} else {
51-
t2 = $[5];
52-
}
53-
const mapped = t2;
37+
const mapped = t0;
5438
return mapped;
5539
}
5640
function _temp(item_0) {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-map-noAlias-escaping-function.expect.md

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,18 @@ export const FIXTURE_ENTRYPOINT = {
2121
```javascript
2222
import { c as _c } from "react/compiler-runtime";
2323
function Component(props) {
24-
const $ = _c(4);
24+
const $ = _c(2);
2525
const f = _temp;
2626
let t0;
2727
if ($[0] !== props.items) {
28-
t0 = [...props.items].map(f);
28+
const x = [...props.items].map(f);
29+
t0 = [x, f];
2930
$[0] = props.items;
3031
$[1] = t0;
3132
} else {
3233
t0 = $[1];
3334
}
34-
const x = t0;
35-
let t1;
36-
if ($[2] !== x) {
37-
t1 = [x, f];
38-
$[2] = x;
39-
$[3] = t1;
40-
} else {
41-
t1 = $[3];
42-
}
43-
return t1;
35+
return t0;
4436
}
4537
function _temp(item) {
4638
return item;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-arrow-function-1.expect.md

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,19 @@ export const FIXTURE_ENTRYPOINT = {
2323
```javascript
2424
import { c as _c } from "react/compiler-runtime";
2525
function component(a) {
26-
const $ = _c(4);
26+
const $ = _c(2);
2727
let t0;
2828
if ($[0] !== a) {
29-
t0 = { a };
29+
const z = { a };
30+
t0 = () => {
31+
console.log(z);
32+
};
3033
$[0] = a;
3134
$[1] = t0;
3235
} else {
3336
t0 = $[1];
3437
}
35-
const z = t0;
36-
let t1;
37-
if ($[2] !== z) {
38-
t1 = () => {
39-
console.log(z);
40-
};
41-
$[2] = z;
42-
$[3] = t1;
43-
} else {
44-
t1 = $[3];
45-
}
46-
const x = t1;
38+
const x = t0;
4739
return x;
4840
}
4941

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-1.expect.md

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,19 @@ export const FIXTURE_ENTRYPOINT = {
2323
```javascript
2424
import { c as _c } from "react/compiler-runtime";
2525
function component(a) {
26-
const $ = _c(4);
26+
const $ = _c(2);
2727
let t0;
2828
if ($[0] !== a) {
29-
t0 = { a };
29+
const z = { a };
30+
t0 = function () {
31+
console.log(z);
32+
};
3033
$[0] = a;
3134
$[1] = t0;
3235
} else {
3336
t0 = $[1];
3437
}
35-
const z = t0;
36-
let t1;
37-
if ($[2] !== z) {
38-
t1 = function () {
39-
console.log(z);
40-
};
41-
$[2] = z;
42-
$[3] = t1;
43-
} else {
44-
t1 = $[3];
45-
}
46-
const x = t1;
38+
const x = t0;
4739
return x;
4840
}
4941

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-2-iife.expect.md

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,17 @@ export const FIXTURE_ENTRYPOINT = {
2525
```javascript
2626
import { c as _c } from "react/compiler-runtime";
2727
function bar(a) {
28-
const $ = _c(4);
29-
let t0;
30-
if ($[0] !== a) {
31-
t0 = [a];
32-
$[0] = a;
33-
$[1] = t0;
34-
} else {
35-
t0 = $[1];
36-
}
37-
const x = t0;
28+
const $ = _c(2);
3829
let y;
39-
if ($[2] !== x[0][1]) {
30+
if ($[0] !== a) {
31+
const x = [a];
4032
y = {};
4133

4234
y = x[0][1];
43-
$[2] = x[0][1];
44-
$[3] = y;
35+
$[0] = a;
36+
$[1] = y;
4537
} else {
46-
y = $[3];
38+
y = $[1];
4739
}
4840
return y;
4941
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-3-iife.expect.md

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,29 +29,20 @@ export const FIXTURE_ENTRYPOINT = {
2929
```javascript
3030
import { c as _c } from "react/compiler-runtime";
3131
function bar(a, b) {
32-
const $ = _c(6);
33-
let t0;
34-
if ($[0] !== a || $[1] !== b) {
35-
t0 = [a, b];
36-
$[0] = a;
37-
$[1] = b;
38-
$[2] = t0;
39-
} else {
40-
t0 = $[2];
41-
}
42-
const x = t0;
32+
const $ = _c(3);
4333
let y;
44-
if ($[3] !== x[0][1] || $[4] !== x[1][0]) {
34+
if ($[0] !== a || $[1] !== b) {
35+
const x = [a, b];
4536
y = {};
4637
let t = {};
4738

4839
y = x[0][1];
4940
t = x[1][0];
50-
$[3] = x[0][1];
51-
$[4] = x[1][0];
52-
$[5] = y;
41+
$[0] = a;
42+
$[1] = b;
43+
$[2] = y;
5344
} else {
54-
y = $[5];
45+
y = $[2];
5546
}
5647
return y;
5748
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-4-iife.expect.md

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,17 @@ export const FIXTURE_ENTRYPOINT = {
2525
```javascript
2626
import { c as _c } from "react/compiler-runtime";
2727
function bar(a) {
28-
const $ = _c(4);
29-
let t0;
30-
if ($[0] !== a) {
31-
t0 = [a];
32-
$[0] = a;
33-
$[1] = t0;
34-
} else {
35-
t0 = $[1];
36-
}
37-
const x = t0;
28+
const $ = _c(2);
3829
let y;
39-
if ($[2] !== x[0].a[1]) {
30+
if ($[0] !== a) {
31+
const x = [a];
4032
y = {};
4133

4234
y = x[0].a[1];
43-
$[2] = x[0].a[1];
44-
$[3] = y;
35+
$[0] = a;
36+
$[1] = y;
4537
} else {
46-
y = $[3];
38+
y = $[1];
4739
}
4840
return y;
4941
}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-iife.expect.md

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,25 +24,17 @@ export const FIXTURE_ENTRYPOINT = {
2424
```javascript
2525
import { c as _c } from "react/compiler-runtime";
2626
function bar(a) {
27-
const $ = _c(4);
28-
let t0;
29-
if ($[0] !== a) {
30-
t0 = [a];
31-
$[0] = a;
32-
$[1] = t0;
33-
} else {
34-
t0 = $[1];
35-
}
36-
const x = t0;
27+
const $ = _c(2);
3728
let y;
38-
if ($[2] !== x[0]) {
29+
if ($[0] !== a) {
30+
const x = [a];
3931
y = {};
4032

4133
y = x[0];
42-
$[2] = x[0];
43-
$[3] = y;
34+
$[0] = a;
35+
$[1] = y;
4436
} else {
45-
y = $[3];
37+
y = $[1];
4638
}
4739
return y;
4840
}

0 commit comments

Comments
 (0)