Facebook/react
1
0
Fork 0
mirror of https://github.com/facebook/react.git synced 2025-11-01 09:12:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Dont merge if scope output is not guaranteed to change

Browse Source 
As noted earlier in the stack and in chat, there are some cases where the output 
of a reactive scope is not guaranteed to change just because its inputs did. 
Consider a function `foo(x) { return x < 10 }`. If x was 0 and changes to 1, the 
result of `foo(x)` won't change. 

For code such as `[foo(x)]`, then, merging the scope for `t0 = foo(x)` and `t1 = 
[t0]` into a single `t1 = [foo(x)]` could cause us to invalidate `t1` 
unnecessarily. For example, x changing from 0 to 1 would allocate a new array of 
`[true]` even though the value didn't meaningfully change. This is the second 
category of merging, where we merge scopes A and B if the outputs of A are the 
inputs to B. 

With this change, we only do this type of merge if the outputs of the first 
scope are known to invalidate whenever the input does. We're conservative about 
this, and only consider function expressions, arrays, object, and jsx to always 
invalidate. Function calls _may_ invalidate, but as w the `foo()` example here 
they also may not. 

Note that this is purely about optimization and not correctness. We could always 
merge in this case (per earlier in the stack) but that might invalidate more 
often than we would like.
This commit is contained in:
Joe Savona
2023-09-27 14:04:01 -04:00
parent 66841aa9ac
commit 687ad02e43
14 changed files with 321 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/MergeConsecutiveScopes.ts
+63 -7
View File
@@ -11,6 +11,7 @@ import {
Place,
ReactiveBlock,
ReactiveFunction,
ReactiveInstruction,
ReactiveScope,
ReactiveScopeBlock,
ReactiveScopeDependency,
@@ -161,7 +162,7 @@ class Transform extends ReactiveFunctionTransform<void> {
} else {
if (
currentScope !== null &&
canMergeScopes(currentScope.scope.scope, instr.scope) &&
canMergeScopes(currentScope.scope, instr) &&
// If there are intermediate instructions, we can only merge the scopes
// if those intermediate instructions are all used by the second scope.
// if not, merging them would make those values unavailable to subsequent
@@ -243,24 +244,33 @@ function areLValuesLastUsedByScope(
return true;
}
function canMergeScopes(a: ReactiveScope, b: ReactiveScope): boolean {
function canMergeScopes(a: ReactiveScopeBlock, b: ReactiveScopeBlock): boolean {
// Don't merge scopes with reassignments
if (a.reassignments.size !== 0 || b.reassignments.size !== 0) {
if (a.scope.reassignments.size !== 0 || b.scope.reassignments.size !== 0) {
return false;
}
if (areEqualDependencies(a.dependencies, b.dependencies)) {
// Merge scopes whose dependencies are identical
if (areEqualDependencies(a.scope.dependencies, b.scope.dependencies)) {
return true;
}
// Merge scopes where the outputs of the previous scope are the inputs
// of the subsequent scope. Note that the output of a scope is not
// guaranteed to change when its inputs change, for example `foo(x)`
// may not change when `x` changes, for example `foo(x) { return x < 10}`
// will not change as x changes from 0 -> 1.
// Therefore we check that the outputs of the previous scope are of a type
// that is guaranteed to invalidate with its inputs, and only merge in this case.
if (
areEqualDependencies(
new Set(
[...a.declarations.values()].map((declaration) => ({
[...a.scope.declarations.values()].map((declaration) => ({
identifier: declaration.identifier,
path: [],
}))
),
b.dependencies
)
b.scope.dependencies
) &&
scopeAlwaysInvalidatesOnDependencyChanges(a)
) {
return true;
}
@@ -295,3 +305,49 @@ function areEqualDependencies(
function areEqualPaths(a: Array<string>, b: Array<string>): boolean {
return a.length === b.length && a.every((item, ix) => item === b[ix]);
}
function scopeAlwaysInvalidatesOnDependencyChanges(
scope: ReactiveScopeBlock
): boolean {
const visitor = new DeclarationTypeVisitor(scope.scope);
visitor.visitScope(scope, undefined);
return visitor.alwaysInvalidatesOnInputChange;
}
class DeclarationTypeVisitor extends ReactiveFunctionVisitor<void> {
scope: ReactiveScope;
alwaysInvalidatesOnInputChange: boolean = false;
constructor(scope: ReactiveScope) {
super();
this.scope = scope;
}
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
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;
}
}
}
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/allocating-primitive-as-dep-nested-scope.expect.md
+25 -12
View File
@@ -24,31 +24,44 @@ import { unstable_useMemoCache as useMemoCache } from "react"; // bar(props.b) i
// Correctness:
// - y depends on either bar(props.b) or bar(props.b) + 1
function AllocatingPrimitiveAsDepNested(props) {
const $ = useMemoCache(5);
const $ = useMemoCache(9);
const c_0 = $[0] !== props.b;
const c_1 = $[1] !== props.a;
let t2;
let x;
let y;
if (c_0 || c_1) {
const x = {};
x = {};
mutate(x);
const t0 = bar(props.b) + 1;
const c_3 = $[3] !== t0;
const c_4 = $[4] !== t0;
let t1;
if (c_3) {
if (c_4) {
t1 = foo(t0);
$[3] = t0;
$[4] = t1;
$[4] = t0;
$[5] = t1;
} else {
t1 = $[4];
t1 = $[5];
}
const y = t1;
y = t1;
mutate(x, props.a);
t2 = [x, y];
$[0] = props.b;
$[1] = props.a;
$[2] = t2;
$[2] = x;
$[3] = y;
} else {
t2 = $[2];
x = $[2];
y = $[3];
}
const c_6 = $[6] !== x;
const c_7 = $[7] !== y;
let t2;
if (c_6 || c_7) {
t2 = [x, y];
$[6] = x;
$[7] = y;
$[8] = t2;
} else {
t2 = $[8];
}
return t2;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/array-access-assignment.expect.md
+25 -12
View File
@@ -24,33 +24,46 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react";
function foo(a, b, c) {
const $ = useMemoCache(6);
const $ = useMemoCache(10);
const c_0 = $[0] !== a;
const c_1 = $[1] !== b;
const c_2 = $[2] !== c;
let t1;
let x;
let z;
if (c_0 || c_1 || c_2) {
const x = [a];
const c_4 = $[4] !== b;
x = [a];
const c_5 = $[5] !== b;
let t0;
if (c_4) {
if (c_5) {
t0 = [null, b];
$[4] = b;
$[5] = t0;
$[5] = b;
$[6] = t0;
} else {
t0 = $[5];
t0 = $[6];
}
const y = t0;
const z = [[], [], [c]];
z = [[], [], [c]];
x[0] = y[1];
z[0][0] = x[0];
t1 = [x, z];
$[0] = a;
$[1] = b;
$[2] = c;
$[3] = t1;
$[3] = x;
$[4] = z;
} else {
t1 = $[3];
x = $[3];
z = $[4];
}
const c_7 = $[7] !== x;
const c_8 = $[8] !== z;
let t1;
if (c_7 || c_8) {
t1 = [x, z];
$[7] = x;
$[8] = z;
$[9] = t1;
} else {
t1 = $[9];
}
return t1;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/array-at-effect.expect.md
+31 -22
View File
@@ -22,38 +22,47 @@ import { unstable_useMemoCache as useMemoCache } from "react"; // arrayInstance.
// - read on receiver
// - mutate on lvalue
function ArrayAtTest(props) {
const $ = useMemoCache(7);
const $ = useMemoCache(9);
const c_0 = $[0] !== props.x;
let t0;
if (c_0) {
t0 = [foo(props.x)];
t0 = foo(props.x);
$[0] = props.x;
$[1] = t0;
} else {
t0 = $[1];
}
const arr = t0;
const c_2 = $[2] !== props.y;
const c_3 = $[3] !== arr;
let t2;
if (c_2 || c_3) {
const c_5 = $[5] !== props.y;
let t1;
if (c_5) {
t1 = bar(props.y);
$[5] = props.y;
$[6] = t1;
} else {
t1 = $[6];
}
t2 = arr.at(t1);
$[2] = props.y;
$[3] = arr;
$[4] = t2;
const c_2 = $[2] !== t0;
let t1;
if (c_2) {
t1 = [t0];
$[2] = t0;
$[3] = t1;
} else {
t2 = $[4];
t1 = $[3];
}
const result = t2;
const arr = t1;
const c_4 = $[4] !== props.y;
const c_5 = $[5] !== arr;
let t3;
if (c_4 || c_5) {
const c_7 = $[7] !== props.y;
let t2;
if (c_7) {
t2 = bar(props.y);
$[7] = props.y;
$[8] = t2;
} else {
t2 = $[8];
}
t3 = arr.at(t2);
$[4] = props.y;
$[5] = arr;
$[6] = t3;
} else {
t3 = $[6];
}
const result = t3;
return result;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/codegen-emit-make-read-only.expect.md
+15 -8
View File
@@ -23,22 +23,29 @@ import { makeReadOnly } from "react-forget-runtime";
import { unstable_useMemoCache as useMemoCache } from "react"; // @enableEmitFreeze true
function MyComponentName(props) {
const $ = useMemoCache(3);
const $ = useMemoCache(5);
const c_0 = $[0] !== props.a;
const c_1 = $[1] !== props.b;
let y;
let x;
if (c_0 || c_1) {
const x = {};
x = {};
foo(x, props.a);
foo(x, props.b);
y = [];
y.push(x);
$[0] = props.a;
$[1] = props.b;
$[2] = __DEV__ ? makeReadOnly(y, "MyComponentName") : y;
$[2] = __DEV__ ? makeReadOnly(x, "MyComponentName") : x;
} else {
y = $[2];
x = $[2];
}
const c_3 = $[3] !== x;
let y;
if (c_3) {
y = [];
y.push(x);
$[3] = x;
$[4] = __DEV__ ? makeReadOnly(y, "MyComponentName") : y;
} else {
y = $[4];
}
return y;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/fbt-template-string-same-scope.expect.md
+14 -9
View File
@@ -29,7 +29,7 @@ import { unstable_useMemoCache as useMemoCache } from "react";
import fbt from "fbt";
export function Component(props) {
const $ = useMemoCache(2);
const $ = useMemoCache(4);
let count = 0;
if (props.items) {
count = props.items.length;
@@ -37,19 +37,24 @@ export function Component(props) {
const c_0 = $[0] !== count;
let t0;
if (c_0) {
t0 = (
<View>
{fbt._("for {count} experiences", [fbt._param("count", count)], {
hk: "nmYpm",
})}
</View>
);
t0 = fbt._("for {count} experiences", [fbt._param("count", count)], {
hk: "nmYpm",
});
$[0] = count;
$[1] = t0;
} else {
t0 = $[1];
}
return t0;
const c_2 = $[2] !== t0;
let t1;
if (c_2) {
t1 = <View>{t0}</View>;
$[2] = t0;
$[3] = t1;
} else {
t1 = $[3];
}
return t1;
}
```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/for-in-statement.expect.md
+14 -7
View File
@@ -22,20 +22,27 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react";
function Component(props) {
const $ = useMemoCache(2);
const $ = useMemoCache(4);
const c_0 = $[0] !== props;
let t0;
let items;
if (c_0) {
const items = [];
items = [];
for (const key in props) {
items.push(<div key={key}>{key}</div>);
}
t0 = <div>{items}</div>;
$[0] = props;
$[1] = t0;
$[1] = items;
} else {
t0 = $[1];
items = $[1];
}
const c_2 = $[2] !== items;
let t0;
if (c_2) {
t0 = <div>{items}</div>;
$[2] = items;
$[3] = t0;
} else {
t0 = $[3];
}
return t0;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/phi-type-inference-array-push.expect.md
+20 -8
View File
@@ -32,12 +32,13 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react"; // @debug
function Component(props) {
const $ = useMemoCache(2);
const $ = useMemoCache(6);
const c_0 = $[0] !== props;
let t0;
let x;
let y;
if (c_0) {
const x = {};
let y = undefined;
x = {};
y = undefined;
if (props.cond) {
y = [props.value];
} else {
@@ -45,12 +46,23 @@ function Component(props) {
}
y.push(x);
t0 = [x, y];
$[0] = props;
$[1] = t0;
$[1] = x;
$[2] = y;
} else {
t0 = $[1];
x = $[1];
y = $[2];
}
const c_3 = $[3] !== x;
const c_4 = $[4] !== y;
let t0;
if (c_3 || c_4) {
t0 = [x, y];
$[3] = x;
$[4] = y;
$[5] = t0;
} else {
t0 = $[5];
}
return t0;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/primitive-as-dep-nested-scope.expect.md
+25 -12
View File
@@ -26,31 +26,44 @@ import { unstable_useMemoCache as useMemoCache } from "react"; // props.b + 1 is
// Correctness:
// y depends on either props.b or props.b + 1
function PrimitiveAsDepNested(props) {
const $ = useMemoCache(5);
const $ = useMemoCache(9);
const c_0 = $[0] !== props.b;
const c_1 = $[1] !== props.a;
let t2;
let x;
let y;
if (c_0 || c_1) {
const x = {};
x = {};
mutate(x);
const t0 = props.b + 1;
const c_3 = $[3] !== t0;
const c_4 = $[4] !== t0;
let t1;
if (c_3) {
if (c_4) {
t1 = foo(t0);
$[3] = t0;
$[4] = t1;
$[4] = t0;
$[5] = t1;
} else {
t1 = $[4];
t1 = $[5];
}
const y = t1;
y = t1;
mutate(x, props.a);
t2 = [x, y];
$[0] = props.b;
$[1] = props.a;
$[2] = t2;
$[2] = x;
$[3] = y;
} else {
t2 = $[2];
x = $[2];
y = $[3];
}
const c_6 = $[6] !== x;
const c_7 = $[7] !== y;
let t2;
if (c_6 || c_7) {
t2 = [x, y];
$[6] = x;
$[7] = y;
$[8] = t2;
} else {
t2 = $[8];
}
return t2;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-scopes.expect.md
+14 -7
View File
@@ -26,24 +26,31 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react";
function f(a, b) {
const $ = useMemoCache(3);
const $ = useMemoCache(5);
const c_0 = $[0] !== a.length;
const c_1 = $[1] !== b;
let t0;
let x;
if (c_0 || c_1) {
const x = [];
x = [];
if (a.length === 1) {
if (b) {
x.push(b);
}
}
t0 = <div>{x}</div>;
$[0] = a.length;
$[1] = b;
$[2] = t0;
$[2] = x;
} else {
t0 = $[2];
x = $[2];
}
const c_3 = $[3] !== x;
let t0;
if (c_3) {
t0 = <div>{x}</div>;
$[3] = x;
$[4] = t0;
} else {
t0 = $[4];
}
return t0;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-analysis-interleaved-reactivity.expect.md
+15 -11
View File
@@ -35,33 +35,37 @@ export const FIXTURE_ENTRYPOINT = {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react";
function Component(props) {
const $ = useMemoCache(5);
const $ = useMemoCache(6);
const c_0 = $[0] !== props.b;
let a;
let t0;
if (c_0) {
a = {};
const b = [];
b.push(props.b);
a.a = null;
t0 = [a];
$[0] = props.b;
$[1] = a;
$[2] = t0;
} else {
a = $[1];
t0 = $[2];
}
const c_2 = $[2] !== a;
let t0;
if (c_2) {
t0 = [a];
$[2] = a;
$[3] = t0;
} else {
t0 = $[3];
}
const c = t0;
const c_3 = $[3] !== a;
const c_4 = $[4] !== a;
let t1;
if (c_3) {
if (c_4) {
t1 = [c, a];
$[3] = a;
$[4] = t1;
$[4] = a;
$[5] = t1;
} else {
t1 = $[4];
t1 = $[5];
}
return t1;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reduce-reactive-deps-join-uncond-scopes-cond-deps.expect.md
+24 -12
View File
@@ -52,30 +52,42 @@ import { unstable_useMemoCache as useMemoCache } from "react"; // This tests an
// }
function TestJoinCondDepsInUncondScopes(props) {
const $ = useMemoCache(4);
const $ = useMemoCache(8);
const c_0 = $[0] !== props.a.b;
let t0;
let x;
let y;
if (c_0) {
const y = {};
const c_2 = $[2] !== props;
let x;
if (c_2) {
y = {};
const c_3 = $[3] !== props;
if (c_3) {
x = {};
if (foo) {
mutate1(x, props.a.b);
}
$[2] = props;
$[3] = x;
$[3] = props;
$[4] = x;
} else {
x = $[3];
x = $[4];
}
mutate2(y, props.a.b);
t0 = [x, y];
$[0] = props.a.b;
$[1] = t0;
$[1] = x;
$[2] = y;
} else {
t0 = $[1];
x = $[1];
y = $[2];
}
const c_5 = $[5] !== x;
const c_6 = $[6] !== y;
let t0;
if (c_5 || c_6) {
t0 = [x, y];
$[5] = x;
$[6] = y;
$[7] = t0;
} else {
t0 = $[7];
}
return t0;
}
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/temporary-accessed-outside-scope.expect.md
+18 -3
View File
@@ -15,19 +15,34 @@ function Component(props) {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react";
function Component(props) {
const $ = useMemoCache(2);
const $ = useMemoCache(6);
const c_0 = $[0] !== props;
let t0;
let t1;
if (c_0) {
const maybeMutable = new MaybeMutable();
const x = props;
t0 = [x, maybeMutate(maybeMutable)];
t0 = x;
t1 = maybeMutate(maybeMutable);
$[0] = props;
$[1] = t0;
$[2] = t1;
} else {
t0 = $[1];
t1 = $[2];
}
return t0;
const c_3 = $[3] !== t0;
const c_4 = $[4] !== t1;
let t2;
if (c_3 || c_4) {
t2 = [t0, t1];
$[3] = t0;
$[4] = t1;
$[5] = t2;
} else {
t2 = $[5];
}
return t2;
}
```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/temporary-property-load-accessed-outside-scope.expect.md
+18 -3
View File
@@ -15,19 +15,34 @@ function Component(props) {
```javascript
import { unstable_useMemoCache as useMemoCache } from "react";
function Component(props) {
const $ = useMemoCache(2);
const $ = useMemoCache(6);
const c_0 = $[0] !== props.value;
let t0;
let t1;
if (c_0) {
const maybeMutable = new MaybeMutable();
const x = props.value;
t0 = [x, maybeMutate(maybeMutable)];
t0 = x;
t1 = maybeMutate(maybeMutable);
$[0] = props.value;
$[1] = t0;
$[2] = t1;
} else {
t0 = $[1];
t1 = $[2];
}
return t0;
const c_3 = $[3] !== t0;
const c_4 = $[4] !== t1;
let t2;
if (c_3 || c_4) {
t2 = [t0, t1];
$[3] = t0;
$[4] = t1;
$[5] = t2;
} else {
t2 = $[5];
}
return t2;
}
```