Skip to content

Commit

Permalink
compiler: Use types to decide which scopes are eligible for merging
Browse files Browse the repository at this point in the history
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: 22e49606056cd37220892cc2298b1318147b97da
Pull Request resolved: #29157
  • Loading branch information
josephsavona committed May 18, 2024
1 parent 296e703 commit 8fc76d7
Show file tree
Hide file tree
Showing 21 changed files with 262 additions and 501 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
Place,
ReactiveBlock,
ReactiveFunction,
ReactiveInstruction,
ReactiveScope,
ReactiveScopeBlock,
ReactiveScopeDependencies,
Expand Down Expand Up @@ -514,64 +513,7 @@ function scopeIsEligibleForMerging(scopeBlock: ReactiveScopeBlock): boolean {
*/
return true;
}
const visitor = new DeclarationTypeVisitor(scopeBlock.scope);
visitor.visitScope(scopeBlock, undefined);
return visitor.alwaysInvalidatesOnInputChange;
}

class DeclarationTypeVisitor extends ReactiveFunctionVisitor<void> {
scope: ReactiveScope;
alwaysInvalidatesOnInputChange: boolean = false;

constructor(scope: ReactiveScope) {
super();
this.scope = scope;
}

override visitScope(scopeBlock: ReactiveScopeBlock, state: void): void {
if (scopeBlock.scope.id !== this.scope.id) {
return;
}
this.traverseScope(scopeBlock, state);
}

override visitInstruction(
instruction: ReactiveInstruction,
state: void
): void {
this.traverseInstruction(instruction, state);
if (
instruction.lvalue === null ||
!this.scope.declarations.has(instruction.lvalue.identifier.id)
) {
/*
* no lvalue or this instruction isn't directly constructing a
* scope output value, skip
*/
log(
` skip instruction lvalue=${
instruction.lvalue?.identifier.id
} declaration?=${
instruction.lvalue != null &&
this.scope.declarations.has(instruction.lvalue.identifier.id)
} scope=${printReactiveScopeSummary(this.scope)}`
);
return;
}
switch (instruction.value.kind) {
case "FunctionExpression":
case "ArrayExpression":
case "JsxExpression":
case "JsxFragment":
case "ObjectExpression": {
/*
* These instruction types *always* allocate. If they execute
* they will produce a new value, triggering downstream reactive
* updates
*/
this.alwaysInvalidatesOnInputChange = true;
break;
}
}
}
return [...scopeBlock.scope.declarations].some(([, decl]) =>
isAlwaysInvalidatingType(decl.identifier.type)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,39 +24,28 @@ import { c as _c } from "react/compiler-runtime"; // bar(props.b) is an allocati
// Correctness:
// - y depends on either bar(props.b) or bar(props.b) + 1
function AllocatingPrimitiveAsDepNested(props) {
const $ = _c(9);
let x;
let y;
const $ = _c(5);
let t0;
if ($[0] !== props.b || $[1] !== props.a) {
x = {};
const x = {};
mutate(x);
const t0 = bar(props.b) + 1;
let t1;
if ($[4] !== t0) {
t1 = foo(t0);
$[4] = t0;
$[5] = t1;
const t1 = bar(props.b) + 1;
let t2;
if ($[3] !== t1) {
t2 = foo(t1);
$[3] = t1;
$[4] = t2;
} else {
t1 = $[5];
t2 = $[4];
}
y = t1;
const y = t2;
mutate(x, props.a);
t0 = [x, y];
$[0] = props.b;
$[1] = props.a;
$[2] = x;
$[3] = y;
} else {
x = $[2];
y = $[3];
}
let t0;
if ($[6] !== x || $[7] !== y) {
t0 = [x, y];
$[6] = x;
$[7] = y;
$[8] = t0;
$[2] = t0;
} else {
t0 = $[8];
t0 = $[2];
}
return t0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,40 +24,29 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { c as _c } from "react/compiler-runtime";
function foo(a, b, c) {
const $ = _c(10);
let x;
let z;
const $ = _c(6);
let t0;
if ($[0] !== a || $[1] !== b || $[2] !== c) {
x = [a];
let t0;
if ($[5] !== b) {
t0 = [null, b];
$[5] = b;
$[6] = t0;
const x = [a];
let t1;
if ($[4] !== b) {
t1 = [null, b];
$[4] = b;
$[5] = t1;
} else {
t0 = $[6];
t1 = $[5];
}
const y = t0;
z = [[], [], [c]];
const y = t1;
const z = [[], [], [c]];
x[0] = y[1];
z[0][0] = x[0];
t0 = [x, z];
$[0] = a;
$[1] = b;
$[2] = c;
$[3] = x;
$[4] = z;
} else {
x = $[3];
z = $[4];
}
let t0;
if ($[7] !== x || $[8] !== z) {
t0 = [x, z];
$[7] = x;
$[8] = z;
$[9] = t0;
$[3] = t0;
} else {
t0 = $[9];
t0 = $[3];
}
return t0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,20 @@ import { makeReadOnly } from "react-compiler-runtime";
import { c as _c } from "react/compiler-runtime"; // @enableEmitFreeze true

function MyComponentName(props) {
const $ = _c(5);
let x;
const $ = _c(3);
let y;
if ($[0] !== props.a || $[1] !== props.b) {
x = {};
const x = {};
foo(x, props.a);
foo(x, props.b);
$[0] = props.a;
$[1] = props.b;
$[2] = __DEV__ ? makeReadOnly(x, "MyComponentName") : x;
} else {
x = $[2];
}
let y;
if ($[3] !== x) {

y = [];
y.push(x);
$[3] = x;
$[4] = __DEV__ ? makeReadOnly(y, "MyComponentName") : y;
$[0] = props.a;
$[1] = props.b;
$[2] = __DEV__ ? makeReadOnly(y, "MyComponentName") : y;
} else {
y = $[4];
y = $[2];
}
return y;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,23 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { c as _c } from "react/compiler-runtime";
function foo(a, b) {
const $ = _c(5);
let x;
const $ = _c(2);
let y;
if ($[0] !== a) {
x = [];
const x = [];
x.push(a);
$[0] = a;
$[1] = x;
} else {
x = $[1];
}
let y;
if ($[2] !== x || $[3] !== b) {

y = [];
if (x.length) {
y.push(x);
}
if (b) {
y.push(b);
}
$[2] = x;
$[3] = b;
$[4] = y;
$[0] = a;
$[1] = y;
} else {
y = $[4];
y = $[1];
}
return y;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,19 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { c as _c } from "react/compiler-runtime";
function Component(props) {
const $ = _c(4);
let items;
const $ = _c(2);
let t0;
if ($[0] !== props) {
items = [];
const items = [];
for (const key in props) {
items.push(<div key={key}>{key}</div>);
}
$[0] = props;
$[1] = items;
} else {
items = $[1];
}
let t0;
if ($[2] !== items) {

t0 = <div>{items}</div>;
$[2] = items;
$[3] = t0;
$[0] = props;
$[1] = t0;
} else {
t0 = $[3];
t0 = $[1];
}
return t0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,21 @@ export const FIXTURE_ENTRYPOINT = {
import { c as _c } from "react/compiler-runtime";
const TOTAL = 10;
function Component(props) {
const $ = _c(5);
let items;
const $ = _c(3);
let t0;
if ($[0] !== props.start || $[1] !== props.items) {
items = [];
const items = [];
for (let i = props.start ?? 0; i < props.items.length; i++) {
const item = props.items[i];
items.push(<div key={item.id}>{item.value}</div>);
}

t0 = <div>{items}</div>;
$[0] = props.start;
$[1] = props.items;
$[2] = items;
} else {
items = $[2];
}
let t0;
if ($[3] !== items) {
t0 = <div>{items}</div>;
$[3] = items;
$[4] = t0;
$[2] = t0;
} else {
t0 = $[4];
t0 = $[2];
}
return t0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,24 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { c as _c } from "react/compiler-runtime";
function Component(props) {
const $ = _c(6);
let x;
let y;
const $ = _c(2);
let t0;
if ($[0] !== props) {
x = {};
const x = {};
let y;
if (props.cond) {
y = [props.value];
} else {
y = [];
}

y.push(x);
$[0] = props;
$[1] = x;
$[2] = y;
} else {
x = $[1];
y = $[2];
}
let t0;
if ($[3] !== x || $[4] !== y) {

t0 = [x, y];
$[3] = x;
$[4] = y;
$[5] = t0;
$[0] = props;
$[1] = t0;
} else {
t0 = $[5];
t0 = $[1];
}
return t0;
}
Expand Down
Loading

0 comments on commit 8fc76d7

Please sign in to comment.