Skip to content

Commit 735e9ac

Browse files
josephsavonajosephsavona
authored
[compiler] enablePreserveExistingMemo memoizes primitive-returning functions (facebook#34343)
`@enablePreserveExistingMemoizationGuarantees` mode currently does not guarantee memoization of primitive-returning functions. We're often able to infer that a function returns a primitive based on how its result is used, for example `foo() + 1` or `object[getIndex()]`, and by default we do not currently memoize computation that produces a primitive. The reasoning behind this is that the compiler is primarily focused on stopping cascading updates — it's fine to recompute a primitive since we can cheaply compare that primitive and avoid unnecessary downstream recomputation. But we've gotten a lot of feedback that people find this surprising, and that sometimes the computation can be expensive enough that it should be memoized. This PR changes `@enablePreserveExistingMemoizationGuarantees` mode to ensure that primitive-returning functions get memoized. Other modes will not memoize these functions. Separately from this we are considering enabling this mode by default. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34343). * facebook#34347 * facebook#34346 * __->__ facebook#34343 * facebook#34335
1 parent 5d64f74 commit 735e9ac

7 files changed

+376
-4
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,9 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor<
411411
this.state = state;
412412
this.options = {
413413
memoizeJsxElements: !this.env.config.enableForest,
414-
forceMemoizePrimitives: this.env.config.enableForest,
414+
forceMemoizePrimitives:
415+
this.env.config.enableForest ||
416+
this.env.config.enablePreserveExistingMemoizationGuarantees,
415417
};
416418
}
417419

@@ -534,9 +536,23 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor<
534536
case 'JSXText':
535537
case 'BinaryExpression':
536538
case 'UnaryExpression': {
537-
const level = options.forceMemoizePrimitives
538-
? MemoizationLevel.Memoized
539-
: MemoizationLevel.Never;
539+
if (options.forceMemoizePrimitives) {
540+
/**
541+
* Because these instructions produce primitives we usually don't consider
542+
* them as escape points: they are known to copy, not return references.
543+
* However if we're forcing memoization of primitives then we mark these
544+
* instructions as needing memoization and walk their rvalues to ensure
545+
* any scopes transitively reachable from the rvalues are considered for
546+
* memoization. Note: we may still prune primitive-producing scopes if
547+
* they don't ultimately escape at all.
548+
*/
549+
const level = MemoizationLevel.Memoized;
550+
return {
551+
lvalues: lvalue !== null ? [{place: lvalue, level}] : [],
552+
rvalues: [...eachReactiveValueOperand(value)],
553+
};
554+
}
555+
const level = MemoizationLevel.Never;
540556
return {
541557
// All of these instructions return a primitive value and never need to be memoized
542558
lvalues: lvalue !== null ? [{place: lvalue, level}] : [],

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.expect.md‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
6+
import {useMemo} from 'react';
7+
import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime';
8+
9+
function Component(props) {
10+
const result = useMemo(
11+
() => makeObject(props.value).value + 1,
12+
[props.value]
13+
);
14+
console.log(result);
15+
return 'ok';
16+
}
17+
18+
function makeObject(value) {
19+
console.log(value);
20+
return {value};
21+
}
22+
23+
export const TODO_FIXTURE_ENTRYPOINT = {
24+
fn: Component,
25+
params: [{value: 42}],
26+
sequentialRenders: [
27+
{value: 42},
28+
{value: 42},
29+
{value: 3.14},
30+
{value: 3.14},
31+
{value: 42},
32+
{value: 3.14},
33+
{value: 42},
34+
{value: 3.14},
35+
],
36+
};
37+
38+
```
39+
40+
## Code
41+
42+
```javascript
43+
// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
44+
import { useMemo } from "react";
45+
import { makeObject_Primitives, ValidateMemoization } from "shared-runtime";
46+
47+
function Component(props) {
48+
const result = makeObject(props.value).value + 1;
49+
50+
console.log(result);
51+
return "ok";
52+
}
53+
54+
function makeObject(value) {
55+
console.log(value);
56+
return { value };
57+
}
58+
59+
export const TODO_FIXTURE_ENTRYPOINT = {
60+
fn: Component,
61+
params: [{ value: 42 }],
62+
sequentialRenders: [
63+
{ value: 42 },
64+
{ value: 42 },
65+
{ value: 3.14 },
66+
{ value: 3.14 },
67+
{ value: 42 },
68+
{ value: 3.14 },
69+
{ value: 42 },
70+
{ value: 3.14 },
71+
],
72+
};
73+
74+
```
75+
76+
### Eval output
77+
(kind: exception) Fixture not implemented

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping-useMemo.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
2+
import {useMemo} from 'react';
3+
import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime';
4+
5+
function Component(props) {
6+
const result = useMemo(
7+
() => makeObject(props.value).value + 1,
8+
[props.value]
9+
);
10+
console.log(result);
11+
return 'ok';
12+
}
13+
14+
function makeObject(value) {
15+
console.log(value);
16+
return {value};
17+
}
18+
19+
export const TODO_FIXTURE_ENTRYPOINT = {
20+
fn: Component,
21+
params: [{value: 42}],
22+
sequentialRenders: [
23+
{value: 42},
24+
{value: 42},
25+
{value: 3.14},
26+
{value: 3.14},
27+
{value: 42},
28+
{value: 3.14},
29+
{value: 42},
30+
{value: 3.14},
31+
],
32+
};

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.expect.md‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
6+
import {useMemo} from 'react';
7+
import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime';
8+
9+
function Component(props) {
10+
const result = makeObject(props.value).value + 1;
11+
console.log(result);
12+
return 'ok';
13+
}
14+
15+
function makeObject(value) {
16+
console.log(value);
17+
return {value};
18+
}
19+
20+
export const FIXTURE_ENTRYPOINT = {
21+
fn: Component,
22+
params: [{value: 42}],
23+
sequentialRenders: [
24+
{value: 42},
25+
{value: 42},
26+
{value: 3.14},
27+
{value: 3.14},
28+
{value: 42},
29+
{value: 3.14},
30+
{value: 42},
31+
{value: 3.14},
32+
],
33+
};
34+
35+
```
36+
37+
## Code
38+
39+
```javascript
40+
// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
41+
import { useMemo } from "react";
42+
import { makeObject_Primitives, ValidateMemoization } from "shared-runtime";
43+
44+
function Component(props) {
45+
const result = makeObject(props.value).value + 1;
46+
console.log(result);
47+
return "ok";
48+
}
49+
50+
function makeObject(value) {
51+
console.log(value);
52+
return { value };
53+
}
54+
55+
export const FIXTURE_ENTRYPOINT = {
56+
fn: Component,
57+
params: [{ value: 42 }],
58+
sequentialRenders: [
59+
{ value: 42 },
60+
{ value: 42 },
61+
{ value: 3.14 },
62+
{ value: 3.14 },
63+
{ value: 42 },
64+
{ value: 3.14 },
65+
{ value: 42 },
66+
{ value: 3.14 },
67+
],
68+
};
69+
70+
```
71+
72+
### Eval output
73+
(kind: ok) "ok"
74+
"ok"
75+
"ok"
76+
"ok"
77+
"ok"
78+
"ok"
79+
"ok"
80+
"ok"
81+
logs: [42,43,42,43,3.14,4.140000000000001,3.14,4.140000000000001,42,43,3.14,4.140000000000001,42,43,3.14,4.140000000000001]

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/dont-memoize-primitive-function-call-non-escaping.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
2+
import {useMemo} from 'react';
3+
import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime';
4+
5+
function Component(props) {
6+
const result = makeObject(props.value).value + 1;
7+
console.log(result);
8+
return 'ok';
9+
}
10+
11+
function makeObject(value) {
12+
console.log(value);
13+
return {value};
14+
}
15+
16+
export const FIXTURE_ENTRYPOINT = {
17+
fn: Component,
18+
params: [{value: 42}],
19+
sequentialRenders: [
20+
{value: 42},
21+
{value: 42},
22+
{value: 3.14},
23+
{value: 3.14},
24+
{value: 42},
25+
{value: 3.14},
26+
{value: 42},
27+
{value: 3.14},
28+
],
29+
};

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-primitive-function-calls.expect.md‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
6+
import {useMemo} from 'react';
7+
import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime';
8+
9+
function Component(props) {
10+
const result = useMemo(() => {
11+
return makeObject(props.value).value + 1;
12+
}, [props.value]);
13+
return <ValidateMemoization inputs={[props.value]} output={result} />;
14+
}
15+
16+
function makeObject(value) {
17+
console.log(value);
18+
return {value};
19+
}
20+
21+
export const FIXTURE_ENTRYPOINT = {
22+
fn: Component,
23+
params: [{value: 42}],
24+
sequentialRenders: [
25+
{value: 42},
26+
{value: 42},
27+
{value: 3.14},
28+
{value: 3.14},
29+
{value: 42},
30+
{value: 3.14},
31+
{value: 42},
32+
{value: 3.14},
33+
],
34+
};
35+
36+
```
37+
38+
## Code
39+
40+
```javascript
41+
import { c as _c } from "react/compiler-runtime"; // @compilationMode:"infer" @enablePreserveExistingMemoizationGuarantees @validatePreserveExistingMemoizationGuarantees
42+
import { useMemo } from "react";
43+
import { makeObject_Primitives, ValidateMemoization } from "shared-runtime";
44+
45+
function Component(props) {
46+
const $ = _c(7);
47+
let t0;
48+
if ($[0] !== props.value) {
49+
t0 = makeObject(props.value);
50+
$[0] = props.value;
51+
$[1] = t0;
52+
} else {
53+
t0 = $[1];
54+
}
55+
const result = t0.value + 1;
56+
let t1;
57+
if ($[2] !== props.value) {
58+
t1 = [props.value];
59+
$[2] = props.value;
60+
$[3] = t1;
61+
} else {
62+
t1 = $[3];
63+
}
64+
let t2;
65+
if ($[4] !== result || $[5] !== t1) {
66+
t2 = <ValidateMemoization inputs={t1} output={result} />;
67+
$[4] = result;
68+
$[5] = t1;
69+
$[6] = t2;
70+
} else {
71+
t2 = $[6];
72+
}
73+
return t2;
74+
}
75+
76+
function makeObject(value) {
77+
console.log(value);
78+
return { value };
79+
}
80+
81+
export const FIXTURE_ENTRYPOINT = {
82+
fn: Component,
83+
params: [{ value: 42 }],
84+
sequentialRenders: [
85+
{ value: 42 },
86+
{ value: 42 },
87+
{ value: 3.14 },
88+
{ value: 3.14 },
89+
{ value: 42 },
90+
{ value: 3.14 },
91+
{ value: 42 },
92+
{ value: 3.14 },
93+
],
94+
};
95+
96+
```
97+
98+
### Eval output
99+
(kind: ok) <div>{"inputs":[42],"output":43}</div>
100+
<div>{"inputs":[42],"output":43}</div>
101+
<div>{"inputs":[3.14],"output":4.140000000000001}</div>
102+
<div>{"inputs":[3.14],"output":4.140000000000001}</div>
103+
<div>{"inputs":[42],"output":43}</div>
104+
<div>{"inputs":[3.14],"output":4.140000000000001}</div>
105+
<div>{"inputs":[42],"output":43}</div>
106+
<div>{"inputs":[3.14],"output":4.140000000000001}</div>
107+
logs: [42,3.14,42,3.14,42,3.14]

0 commit comments

Comments
 (0)