Infer closures as frozen if they dont capture mutable values
Fix for the previous issue, suggested by @gsathya: when we run InferReferenceEffects on the outer function we check each closure to see if it actually captured any mutable values. If it didn't, we can mark the closure as readonly and memoize it independently.
Joe Savona committed
Apr 4, 2023 at 12:30 UTC
6d62d9f5057f4de89fe06e71aca01974b5f8ae7c
12 files changed
+143
-60
compiler/forget/src/HIR/HIR.ts
+13
@@ -762,6 +762,19 @@ export enum Effect {
762
Store = "store",
763
}
764
765
+export function isMutableEffect(effect: Effect): boolean {
766
+ switch (effect) {
767
+ case Effect.Capture:
768
+ case Effect.Mutate:
769
+ case Effect.Store: {
770
+ return true;
771
+ }
772
+ default: {
773
+ return false;
774
+ }
775
+ }
776
+}
777
+
778
export type ReactiveScope = {
779
id: ScopeId;
780
range: MutableRange;
compiler/forget/src/Inference/InferReferenceEffects.ts
+9
-1
@@ -15,6 +15,7 @@ import {
15
HIRFunction,
16
IdentifierId,
17
InstructionValue,
18
+ isMutableEffect,
19
isObjectType,
20
MethodCall,
21
Phi,
@@ -679,13 +680,20 @@ function inferBlock(
680
break;
681
}
682
case "FunctionExpression": {
683
+ let hasMutableOperand = false;
684
for (const operand of eachInstructionOperand(instr)) {
685
state.reference(
686
operand,
687
operand.effect === Effect.Unknown ? Effect.Read : operand.effect
688
);
689
+ hasMutableOperand ||= isMutableEffect(operand.effect);
690
}
688
- state.initialize(instrValue, ValueKind.Mutable);
691
+ // If a closure did not capture any mutable values, then we can consider it to be
692
+ // frozen, which allows it to be independently memoized.
693
+ state.initialize(
694
+ instrValue,
695
+ hasMutableOperand ? ValueKind.Mutable : ValueKind.Frozen
696
+ );
697
state.define(instr.lvalue, instrValue);
698
instr.lvalue.effect = Effect.Store;
699
continue;
compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts
+1
-14
@@ -9,10 +9,10 @@ import invariant from "invariant";
9
import prettyFormat from "pretty-format";
10
import { CompilerError } from "../CompilerError";
11
import {
12
- Effect,
12
IdentifierId,
13
InstructionId,
14
isHookType,
15
+ isMutableEffect,
16
Pattern,
17
Place,
18
ReactiveFunction,
@@ -702,16 +702,3 @@ class PruneScopesTransform extends ReactiveFunctionTransform<
702
}
703
}
704
}
705
-
706
-function isMutableEffect(effect: Effect): boolean {
707
- switch (effect) {
708
- case Effect.Capture:
709
- case Effect.Mutate:
710
- case Effect.Store: {
711
- return true;
712
- }
713
- default: {
714
- return false;
715
- }
716
- }
717
-}
compiler/forget/src/__tests__/fixtures/compiler/_bug.computed-call-evaluation-order.expect.md
+12
-5
@@ -29,19 +29,26 @@ function changeF(o) {
29
}
30
31
function Component() {
32
- const $ = React.unstable_useMemoCache(1);
33
- let x;
32
+ const $ = React.unstable_useMemoCache(2);
33
+ let t0;
34
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
35
- x = { f: () => console.log("original") };
35
+ t0 = () => console.log("original");
36
+ $[0] = t0;
37
+ } else {
38
+ t0 = $[0];
39
+ }
40
+ let x;
41
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
42
+ x = { f: t0 };
43
44
console.log("A");
45
console.log("B");
46
changeF(x);
47
console.log("arg");
48
x.f(1);
42
- $[0] = x;
49
+ $[1] = x;
50
} else {
44
- x = $[0];
51
+ x = $[1];
52
}
53
return x;
54
}
compiler/forget/src/__tests__/fixtures/compiler/_bug.property-call-evaluation-order.expect.md
+12
-5
@@ -29,18 +29,25 @@ function changeF(o) {
29
}
30
31
function Component() {
32
- const $ = React.unstable_useMemoCache(1);
33
- let x;
32
+ const $ = React.unstable_useMemoCache(2);
33
+ let t0;
34
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
35
- x = { f: () => console.log("original") };
35
+ t0 = () => console.log("original");
36
+ $[0] = t0;
37
+ } else {
38
+ t0 = $[0];
39
+ }
40
+ let x;
41
+ if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
42
+ x = { f: t0 };
43
44
console.log("A");
45
changeF(x);
46
console.log("arg");
47
x.f(1);
41
- $[0] = x;
48
+ $[1] = x;
49
} else {
43
- x = $[0];
50
+ x = $[1];
51
}
52
return x;
53
}
compiler/forget/src/__tests__/fixtures/compiler/array-at-closure.expect.md
+13
-4
@@ -18,7 +18,7 @@ function Component(props) {
18
19
```javascript
20
function Component(props) {
21
- const $ = React.unstable_useMemoCache(5);
21
+ const $ = React.unstable_useMemoCache(7);
22
const c_0 = $[0] !== props.x;
23
let t0;
24
if (c_0) {
@@ -33,18 +33,27 @@ function Component(props) {
33
const c_3 = $[3] !== x;
34
let t1;
35
if (c_2 || c_3) {
36
- const fn = function () {
36
+ t1 = function () {
37
const arr = [...bar(props)];
38
return arr.at(x);
39
};
40
- t1 = fn();
40
$[2] = props;
41
$[3] = x;
42
$[4] = t1;
43
} else {
44
t1 = $[4];
45
}
47
- const fnResult = t1;
46
+ const fn = t1;
47
+ const c_5 = $[5] !== fn;
48
+ let t2;
49
+ if (c_5) {
50
+ t2 = fn();
51
+ $[5] = fn;
52
+ $[6] = t2;
53
+ } else {
54
+ t2 = $[6];
55
+ }
56
+ const fnResult = t2;
57
return fnResult;
58
}
59
compiler/forget/src/__tests__/fixtures/compiler/capturing-function-runs-inference.expect.md
+13
-4
@@ -14,7 +14,7 @@ function component(a, b) {
14
15
```javascript
16
function component(a, b) {
17
- const $ = React.unstable_useMemoCache(4);
17
+ const $ = React.unstable_useMemoCache(6);
18
const c_0 = $[0] !== a;
19
let t0;
20
if (c_0) {
@@ -28,14 +28,23 @@ function component(a, b) {
28
const c_2 = $[2] !== z;
29
let t1;
30
if (c_2) {
31
- const p = () => <Foo>{z}</Foo>;
32
- t1 = p();
31
+ t1 = () => <Foo>{z}</Foo>;
32
$[2] = z;
33
$[3] = t1;
34
} else {
35
t1 = $[3];
36
}
38
- return t1;
37
+ const p = t1;
38
+ const c_4 = $[4] !== p;
39
+ let t2;
40
+ if (c_4) {
41
+ t2 = p();
42
+ $[4] = p;
43
+ $[5] = t2;
44
+ } else {
45
+ t2 = $[5];
46
+ }
47
+ return t2;
48
}
49
50
```
compiler/forget/src/__tests__/fixtures/compiler/function-declaration-simple.expect.md
+11
-4
@@ -17,14 +17,21 @@ function component(a) {
17
18
```javascript
19
function component(a) {
20
- const $ = React.unstable_useMemoCache(2);
20
+ const $ = React.unstable_useMemoCache(3);
21
const c_0 = $[0] !== a;
22
let t;
23
if (c_0) {
24
t = { a };
25
- const x = function x(p) {
26
- p.foo();
27
- };
25
+ let t0;
26
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
27
+ t0 = function x(p) {
28
+ p.foo();
29
+ };
30
+ $[2] = t0;
31
+ } else {
32
+ t0 = $[2];
33
+ }
34
+ const x = t0;
35
x(t);
36
$[0] = a;
37
$[1] = t;
compiler/forget/src/__tests__/fixtures/compiler/inadvertent-mutability-readonly-lambda.expect.md
+21
-5
@@ -22,14 +22,30 @@ function Component(props) {
22
23
```javascript
24
function Component(props) {
25
+ const $ = React.unstable_useMemoCache(4);
26
const [value, setValue] = useState(null);
26
-
27
- const onChange = (e) => setValue((value) => value + e.target.value);
27
+ const c_0 = $[0] !== setValue;
28
+ let t0;
29
+ if (c_0) {
30
+ t0 = (e) => setValue((value) => value + e.target.value);
31
+ $[0] = setValue;
32
+ $[1] = t0;
33
+ } else {
34
+ t0 = $[1];
35
+ }
36
+ const onChange = t0;
37
38
useOtherHook();
30
-
31
- const x = {};
32
- foo(x, onChange);
39
+ const c_2 = $[2] !== onChange;
40
+ let x;
41
+ if (c_2) {
42
+ x = {};
43
+ foo(x, onChange);
44
+ $[2] = onChange;
45
+ $[3] = x;
46
+ } else {
47
+ x = $[3];
48
+ }
49
return x;
50
}
51
compiler/forget/src/__tests__/fixtures/compiler/useEffect-nested-lambdas.expect.md
+21
-11
@@ -5,6 +5,7 @@
5
function Component(props) {
6
const item = useMutable(props.itemId);
7
const dispatch = useDispatch();
8
+ useFreeze(dispatch);
9
10
const exit = useCallback(() => {
11
dispatch(createExitAction());
@@ -30,13 +31,22 @@ function Component(props) {
31
32
```javascript
33
function Component(props) {
33
- const $ = React.unstable_useMemoCache(1);
34
+ const $ = React.unstable_useMemoCache(3);
35
const item = useMutable(props.itemId);
36
const dispatch = useDispatch();
36
-
37
- const exit = () => {
38
- dispatch(createExitAction());
39
- };
37
+ useFreeze(dispatch);
38
+ const c_0 = $[0] !== dispatch;
39
+ let t0;
40
+ if (c_0) {
41
+ t0 = () => {
42
+ dispatch(createExitAction());
43
+ };
44
+ $[0] = dispatch;
45
+ $[1] = t0;
46
+ } else {
47
+ t0 = $[1];
48
+ }
49
+ const exit = t0;
50
51
useEffect(() => {
52
const cleanup = GlobalEventEmitter.addListener("onInput", () => {
@@ -48,14 +58,14 @@ function Component(props) {
58
}, [exit, item]);
59
60
maybeMutate(item);
51
- let t0;
52
- if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
53
- t0 = <div />;
54
- $[0] = t0;
61
+ let t1;
62
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
63
+ t1 = <div />;
64
+ $[2] = t1;
65
} else {
56
- t0 = $[0];
66
+ t1 = $[2];
67
}
58
- return t0;
68
+ return t1;
69
}
70
71
```
compiler/forget/src/__tests__/fixtures/compiler/useEffect-nested-lambdas.js
+1
@@ -1,6 +1,7 @@
1
function Component(props) {
2
const item = useMutable(props.itemId);
3
const dispatch = useDispatch();
4
+ useFreeze(dispatch);
5
6
const exit = useCallback(() => {
7
dispatch(createExitAction());
compiler/forget/src/__tests__/fixtures/compiler/useMemo-simple.expect.md
+16
-7
@@ -13,27 +13,36 @@ function component(a) {
13
14
```javascript
15
function component(a) {
16
- const $ = React.unstable_useMemoCache(4);
16
+ const $ = React.unstable_useMemoCache(6);
17
const c_0 = $[0] !== a;
18
let t0;
19
if (c_0) {
20
- t0 = (() => [a])();
20
+ t0 = () => [a];
21
$[0] = a;
22
$[1] = t0;
23
} else {
24
t0 = $[1];
25
}
26
- const x = t0;
27
- const c_2 = $[2] !== x;
26
+ const c_2 = $[2] !== t0;
27
let t1;
28
if (c_2) {
30
- t1 = <Foo x={x} />;
31
- $[2] = x;
29
+ t1 = t0();
30
+ $[2] = t0;
31
$[3] = t1;
32
} else {
33
t1 = $[3];
34
}
36
- return t1;
35
+ const x = t1;
36
+ const c_4 = $[4] !== x;
37
+ let t2;
38
+ if (c_4) {
39
+ t2 = <Foo x={x} />;
40
+ $[4] = x;
41
+ $[5] = t2;
42
+ } else {
43
+ t2 = $[5];
44
+ }
45
+ return t2;
46
}
47
48
```