@samitouri / QOS-React / commits / a532d91d01

compiler: Known hooks/nonescaping scopes dont count as pruned

There are two cases where it's legit/intended to remove scopes, and we can inline the scope rather than reify a "pruned" scope: * Scopes that contain a single instruction with a hook call. The fact that we create a scope in this case at all is just an artifact of it being simpler to do this and remove the scope later rather than try to avoid creating it in the first place. So for these scopes, we can just inline them. * Scopes that are provably non-escaping. Removing the scope is an optimization, not a case of us having to prune away something that should be there. So again, its fine to inline in this case. I found this from syncing the stack internally and looking at differences in compiled output. The latter case was most common but the first case is just an obvious improvement. ghstack-source-id: 80610ddafad65eb837d0037e2692dd74bc548088 Pull Request resolved: https://github.com/facebook/react/pull/29820

Joe Savona committed Jun 10, 2024 at 08:35 UTC a532d91d010a11083e598027cbd40fc0c935ea92
8 files changed +162 -55
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts
+2 -23
@@ -331,29 +331,8 @@ class CountMemoBlockVisitor extends ReactiveFunctionVisitor<void> {
331 scopeBlock: PrunedReactiveScopeBlock,
332 state: void
333 ): void {
334 - let isHookOnlyMemoBlock = false;
335 - if (
336 - scopeBlock.instructions.length === 1 &&
337 - scopeBlock.instructions[0].kind === "instruction"
338 - ) {
339 - const instr = scopeBlock.instructions[0]!.instruction;
340 - if (
341 - instr.value.kind === "MethodCall" ||
342 - instr.value.kind === "CallExpression"
343 - ) {
344 - const callee =
345 - instr.value.kind === "MethodCall"
346 - ? instr.value.property
347 - : instr.value.callee;
348 - if (getHookKind(this.env, callee.identifier) != null) {
349 - isHookOnlyMemoBlock = true;
350 - }
351 - }
352 - }
353 - if (!isHookOnlyMemoBlock) {
354 - this.prunedMemoBlocks += 1;
355 - this.prunedMemoValues += scopeBlock.scope.declarations.size;
356 - }
334 + this.prunedMemoBlocks += 1;
335 + this.prunedMemoValues += scopeBlock.scope.declarations.size;
336 this.traversePrunedScope(scopeBlock, state);
337 }
338 }
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/FlattenScopesWithHooksOrUse.ts
+14
@@ -66,6 +66,20 @@ class Transform extends ReactiveFunctionTransform<State> {
66 this.visitScope(scope, innerState);
67 outerState.hasHook ||= innerState.hasHook;
68 if (innerState.hasHook) {
69 + if (scope.instructions.length === 1) {
70 + /*
71 + * This was a scope just for a hook call, which doesn't need memoization.
72 + * flatten it away
73 + */
74 + return {
75 + kind: "replace-many",
76 + value: scope.instructions,
77 + };
78 + }
79 + /*
80 + * else this scope had multiple instructions and produced some other value:
81 + * mark it as pruned
82 + */
83 return {
84 kind: "replace",
85 value: {
compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts
+2 -6
@@ -952,12 +952,8 @@ class PruneScopesTransform extends ReactiveFunctionTransform<
952 } else {
953 this.prunedScopes.add(scopeBlock.scope.id);
954 return {
955 - kind: "replace",
956 - value: {
957 - kind: "pruned-scope",
958 - scope: scopeBlock.scope,
959 - instructions: scopeBlock.instructions,
960 - },
955 + kind: "replace-many",
956 + value: scopeBlock.instructions,
957 };
958 }
959 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/log-pruned-memoization.expect.md
+60 -17
@@ -3,10 +3,17 @@
3
4 ```javascript
5 // @logger
6 -import { useState } from "react";
7 -import { identity, makeObject_Primitives, useHook } from "shared-runtime";
6 +import { createContext, use, useState } from "react";
7 +import {
8 + Stringify,
9 + identity,
10 + makeObject_Primitives,
11 + useHook,
12 +} from "shared-runtime";
13
14 function Component() {
15 + const w = use(Context);
16 +
17 // The scopes for x and x2 are interleaved, so this is one scope with two values
18 const x = makeObject_Primitives();
19 const x2 = makeObject_Primitives();
@@ -26,11 +33,21 @@ function Component() {
33 }
34
35 // Overall we expect two pruned scopes (for x+x2, and obj), with 3 pruned scope values.
29 - return [x, x2, y, z];
36 + return <Stringify items={[w, x, x2, y, z]} />;
37 +}
38 +
39 +const Context = createContext();
40 +
41 +function Wrapper() {
42 + return (
43 + <Context value={42}>
44 + <Component />
45 + </Context>
46 + );
47 }
48
49 export const FIXTURE_ENTRYPOINT = {
33 - fn: Component,
50 + fn: Wrapper,
51 params: [{}],
52 };
53
@@ -40,11 +57,17 @@ export const FIXTURE_ENTRYPOINT = {
57
58 ```javascript
59 import { c as _c } from "react/compiler-runtime"; // @logger
43 -import { useState } from "react";
44 -import { identity, makeObject_Primitives, useHook } from "shared-runtime";
60 +import { createContext, use, useState } from "react";
61 +import {
62 + Stringify,
63 + identity,
64 + makeObject_Primitives,
65 + useHook,
66 +} from "shared-runtime";
67
68 function Component() {
47 - const $ = _c(5);
69 + const $ = _c(6);
70 + const w = use(Context);
71
72 const x = makeObject_Primitives();
73 const x2 = makeObject_Primitives();
@@ -65,20 +88,39 @@ function Component() {
88 z = $[0];
89 }
90 let t0;
68 - if ($[1] !== x || $[2] !== x2 || $[3] !== y) {
69 - t0 = [x, x2, y, z];
70 - $[1] = x;
71 - $[2] = x2;
72 - $[3] = y;
73 - $[4] = t0;
91 + if ($[1] !== w || $[2] !== x || $[3] !== x2 || $[4] !== y) {
92 + t0 = <Stringify items={[w, x, x2, y, z]} />;
93 + $[1] = w;
94 + $[2] = x;
95 + $[3] = x2;
96 + $[4] = y;
97 + $[5] = t0;
98 + } else {
99 + t0 = $[5];
100 + }
101 + return t0;
102 +}
103 +
104 +const Context = createContext();
105 +
106 +function Wrapper() {
107 + const $ = _c(1);
108 + let t0;
109 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
110 + t0 = (
111 + <Context value={42}>
112 + <Component />
113 + </Context>
114 + );
115 + $[0] = t0;
116 } else {
75 - t0 = $[4];
117 + t0 = $[0];
118 }
119 return t0;
120 }
121
122 export const FIXTURE_ENTRYPOINT = {
81 - fn: Component,
123 + fn: Wrapper,
124 params: [{}],
125 };
126
@@ -87,8 +129,9 @@ export const FIXTURE_ENTRYPOINT = {
129 ## Logs
130
131 ```
90 -{"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":121},"end":{"line":26,"column":1,"index":813},"filename":"log-pruned-memoization.ts"},"fnName":"Component","memoSlots":5,"memoBlocks":2,"memoValues":2,"prunedMemoBlocks":2,"prunedMemoValues":3}
132 +{"kind":"CompileSuccess","fnLoc":{"start":{"line":10,"column":0,"index":161},"end":{"line":33,"column":1,"index":905},"filename":"log-pruned-memoization.ts"},"fnName":"Component","memoSlots":6,"memoBlocks":2,"memoValues":2,"prunedMemoBlocks":2,"prunedMemoValues":3}
133 +{"kind":"CompileSuccess","fnLoc":{"start":{"line":37,"column":0,"index":941},"end":{"line":43,"column":1,"index":1039},"filename":"log-pruned-memoization.ts"},"fnName":"Wrapper","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
134 ```
135
136 ### Eval output
94 -(kind: ok) [{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},[{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true}]]
\ No newline at end of file
137 +(kind: ok) <div>{"items":[42,{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},[{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true},{"a":0,"b":"value1","c":true}]]}</div>
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/log-pruned-memoization.js
+21 -4
@@ -1,8 +1,15 @@
1 // @logger
2 -import { useState } from "react";
3 -import { identity, makeObject_Primitives, useHook } from "shared-runtime";
2 +import { createContext, use, useState } from "react";
3 +import {
4 + Stringify,
5 + identity,
6 + makeObject_Primitives,
7 + useHook,
8 +} from "shared-runtime";
9
10 function Component() {
11 + const w = use(Context);
12 +
13 // The scopes for x and x2 are interleaved, so this is one scope with two values
14 const x = makeObject_Primitives();
15 const x2 = makeObject_Primitives();
@@ -22,10 +29,20 @@ function Component() {
29 }
30
31 // Overall we expect two pruned scopes (for x+x2, and obj), with 3 pruned scope values.
25 - return [x, x2, y, z];
32 + return <Stringify items={[w, x, x2, y, z]} />;
33 +}
34 +
35 +const Context = createContext();
36 +
37 +function Wrapper() {
38 + return (
39 + <Context value={42}>
40 + <Component />
41 + </Context>
42 + );
43 }
44
45 export const FIXTURE_ENTRYPOINT = {
29 - fn: Component,
46 + fn: Wrapper,
47 params: [{}],
48 };
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-noescaping-dependency-can-inline-into-consuming-scope.expect.md new
+46
@@ -0,0 +1,46 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +// @flow
6 +function Component() {
7 + return (
8 + <div
9 + className={stylex(
10 + // this value is a) in its own scope, b) non-reactive, and c) non-escaping
11 + // its scope gets pruned bc it's non-escaping, but this doesn't mean we need to
12 + // create a temporary for it
13 + flags.feature("feature-name") ? styles.featureNameStyle : null
14 + )}
15 + ></div>
16 + );
17 +}
18 +
19 +```
20 +
21 +## Code
22 +
23 +```javascript
24 +import { c as _c } from "react/compiler-runtime";
25 +function Component() {
26 + const $ = _c(1);
27 + let t0;
28 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
29 + t0 = (
30 + <div
31 + className={stylex(
32 + flags.feature("feature-name") ? styles.featureNameStyle : null,
33 + )}
34 + />
35 + );
36 + $[0] = t0;
37 + } else {
38 + t0 = $[0];
39 + }
40 + return t0;
41 +}
42 +
43 +```
44 +
45 +### Eval output
46 +(kind: exception) Fixture not implemented
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonreactive-noescaping-dependency-can-inline-into-consuming-scope.js new
+13
@@ -0,0 +1,13 @@
1 +// @flow
2 +function Component() {
3 + return (
4 + <div
5 + className={stylex(
6 + // this value is a) in its own scope, b) non-reactive, and c) non-escaping
7 + // its scope gets pruned bc it's non-escaping, but this doesn't mean we need to
8 + // create a temporary for it
9 + flags.feature("feature-name") ? styles.featureNameStyle : null
10 + )}
11 + ></div>
12 + );
13 +}
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-for-of.expect.md
+4 -5
@@ -39,7 +39,7 @@ export const FIXTURE_ENTRYPOINT = {
39 ```javascript
40 import { c as _c } from "react/compiler-runtime";
41 function Component(props) {
42 - const $ = _c(2);
42 + const $ = _c(1);
43
44 const a = [];
45 const b = [];
@@ -53,12 +53,11 @@ function Component(props) {
53 x = 1;
54 }
55 let t0;
56 - if ($[0] !== x) {
56 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
57 t0 = [x];
58 - $[0] = x;
59 - $[1] = t0;
58 + $[0] = t0;
59 } else {
61 - t0 = $[1];
60 + t0 = $[0];
61 }
62 return t0;
63 }