@samitouri / QOS-React-2 / commits / 93e0815b26

Remove unnecessary scopes for value blocks

The example earlier in the stack had unreachable code in the output because there was an unnecessary memoization block around an assignment. This was a holdover from before we moved the logic to expand mutable ranges for phis from LeaveSSA to InferMutableRanges. We were conservatively assigning a mutable range to all variables with a phi, even those that didn't strictly need one. Removing the range extension logic in LeaveSSA fixed the issue, but uncovered the fact that AlignReactiveScopesToBlockScopes was missing a case to handle optionals. ## Test Plan Synced internally and ran a snapshot/comparison of compilation before/after (P1197734337 for those curious). The majority of components get fewer memo slots thanks to not needing to memoize non-allocating value block expressions like ternaries/optionals. In a few cases, the fact that we're no longer assigning a mutable range for value blocks (unless there is actually a mutation!) means we get more fine-grained memoization and increase the number of memoization blocks. So overall this appears to be correct, improve memoization, and reduce code size.

Joe Savona committed Mar 19, 2024 at 13:28 UTC 93e0815b263e2cf341231ae36de0053a4394f586
8 files changed +55 -157
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts
+1
@@ -84,6 +84,7 @@ class Visitor extends ReactiveFunctionVisitor<Context> {
84
85 override visitInstruction(instr: ReactiveInstruction, state: Context): void {
86 switch (instr.value.kind) {
87 + case "OptionalExpression":
88 case "SequenceExpression":
89 case "ConditionalExpression":
90 case "LogicalExpression": {
compiler/packages/babel-plugin-react-forget/src/SSA/LeaveSSA.ts
-45
@@ -14,7 +14,6 @@ import {
14 InstructionKind,
15 LValue,
16 LValuePattern,
17 - makeInstructionId,
17 Phi,
18 Place,
19 } from "../HIR/HIR";
@@ -312,50 +311,6 @@ export function leaveSSA(fn: HIRFunction): void {
311 } else {
312 reassignmentPhis.push({ phi, block: phiBlock });
313 }
315 - const hasBackEdge = backEdgePhis.has(phi);
316 - const isPhiMutatedAfterCreation: boolean =
317 - phi.id.mutableRange.end >
318 - (phiBlock.instructions.at(0)?.id ?? phiBlock.terminal.id);
319 -
320 - /*
321 - * Named variables whose phi doesn't have a back-edge can potentially be independenly
322 - * memoized, depending on whether the phi is after its creation.
323 - */
324 - if (phi.id.name !== null && !hasBackEdge) {
325 - if (!isPhiMutatedAfterCreation) {
326 - /*
327 - * Simple case: predecesor-only values flowing into a phi, which is never modified:
328 - * adjust the phi's range to clarify that the identifier does not mutate
329 - */
330 - phi.id.mutableRange.start = terminal.id;
331 - phi.id.mutableRange.end = makeInstructionId(terminal.id + 1);
332 - } else {
333 - /*
334 - * Predecessor only values flow into a phi, which is modified later:
335 - * all operands flow into the phi and can be modified, must extend their ranges
336 - */
337 - for (const [, operand] of phi.operands) {
338 - operand.mutableRange.end = phi.id.mutableRange.end;
339 - }
340 - }
341 - continue;
342 - }
343 - /*
344 - * Otherwise this is a temporary phi (logical or ternary) or occurs in a loop. In either
345 - * case we can't independently memoize any of the values: unify their ranges to span the
346 - * min(start) to max(end) so that we create a single scope for all the computation.
347 - */
348 - let start = block.terminal.id as number;
349 - let end = Number.MIN_SAFE_INTEGER;
350 - const operands = [phi.id, ...phi.operands.values()];
351 - for (const operand of operands) {
352 - start = Math.min(start, operand.mutableRange.start);
353 - end = Math.max(end, operand.mutableRange.end);
354 - }
355 - for (const operand of operands) {
356 - operand.mutableRange.start = makeInstructionId(start);
357 - operand.mutableRange.end = makeInstructionId(end);
358 - }
314 }
315 }
316 const fallthroughId = terminalFallthrough(terminal);
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md
+22 -30
@@ -56,7 +56,7 @@ function useFragment(_arg1, _arg2) {
56 }
57
58 function Component(props) {
59 - const $ = useMemoCache(16);
59 + const $ = useMemoCache(14);
60 const post = useFragment(graphql`...`, props.post);
61 let media;
62 let allUrls;
@@ -65,48 +65,40 @@ function Component(props) {
65 allUrls = [];
66
67 const { media: t0, comments: t1, urls: t2 } = post;
68 + media = t0 === undefined ? null : t0;
69 let t3;
69 - if ($[4] !== t0) {
70 - t3 = t0 === undefined ? null : t0;
71 - $[4] = t0;
70 + if ($[4] !== t1) {
71 + t3 = t1 === undefined ? [] : t1;
72 + $[4] = t1;
73 $[5] = t3;
74 } else {
75 t3 = $[5];
76 }
76 - media = t3;
77 + const comments = t3;
78 let t4;
78 - if ($[6] !== t1) {
79 - t4 = t1 === undefined ? [] : t1;
80 - $[6] = t1;
79 + if ($[6] !== t2) {
80 + t4 = t2 === undefined ? [] : t2;
81 + $[6] = t2;
82 $[7] = t4;
83 } else {
84 t4 = $[7];
85 }
85 - const comments = t4;
86 + const urls = t4;
87 let t5;
87 - if ($[8] !== t2) {
88 - t5 = t2 === undefined ? [] : t2;
89 - $[8] = t2;
90 - $[9] = t5;
91 - } else {
92 - t5 = $[9];
93 - }
94 - const urls = t5;
95 - let t6;
96 - if ($[10] !== comments.length) {
97 - t6 = (e) => {
88 + if ($[8] !== comments.length) {
89 + t5 = (e) => {
90 if (!comments.length) {
91 return;
92 }
93
94 console.log(comments.length);
95 };
104 - $[10] = comments.length;
105 - $[11] = t6;
96 + $[8] = comments.length;
97 + $[9] = t5;
98 } else {
107 - t6 = $[11];
99 + t5 = $[9];
100 }
109 - onClick = t6;
101 + onClick = t5;
102
103 allUrls.push(...urls);
104 $[0] = post;
@@ -119,14 +111,14 @@ function Component(props) {
111 onClick = $[3];
112 }
113 let t0;
122 - if ($[12] !== media || $[13] !== allUrls || $[14] !== onClick) {
114 + if ($[10] !== media || $[11] !== allUrls || $[12] !== onClick) {
115 t0 = <Stringify media={media} allUrls={allUrls} onClick={onClick} />;
124 - $[12] = media;
125 - $[13] = allUrls;
126 - $[14] = onClick;
127 - $[15] = t0;
116 + $[10] = media;
117 + $[11] = allUrls;
118 + $[12] = onClick;
119 + $[13] = t0;
120 } else {
129 - t0 = $[15];
121 + t0 = $[13];
122 }
123 return t0;
124 }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/escape-analysis-logical.expect.md
+2 -12
@@ -24,7 +24,7 @@ export const FIXTURE_ENTRYPOINT = {
24 ```javascript
25 import { unstable_useMemoCache as useMemoCache } from "react";
26 function Component(props) {
27 - const $ = useMemoCache(10);
27 + const $ = useMemoCache(6);
28 let t0;
29 if ($[0] !== props.a) {
30 t0 = [props.a];
@@ -52,17 +52,7 @@ function Component(props) {
52 t2 = $[5];
53 }
54 const c = t2;
55 - let t3;
56 - if ($[6] !== a || $[7] !== b || $[8] !== c) {
57 - t3 = (a && b) || c;
58 - $[6] = a;
59 - $[7] = b;
60 - $[8] = c;
61 - $[9] = t3;
62 - } else {
63 - t3 = $[9];
64 - }
65 - return t3;
55 + return (a && b) || c;
56 }
57
58 export const FIXTURE_ENTRYPOINT = {
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/optional-computed-member-expression.expect.md
+2 -11
@@ -14,7 +14,7 @@ function Component(props) {
14 ```javascript
15 import { unstable_useMemoCache as useMemoCache } from "react";
16 function Component(props) {
17 - const $ = useMemoCache(5);
17 + const $ = useMemoCache(2);
18 let t0;
19 if ($[0] !== props) {
20 t0 = makeObject(props);
@@ -24,16 +24,7 @@ function Component(props) {
24 t0 = $[1];
25 }
26 const object = t0;
27 - let t1;
28 - if ($[2] !== object || $[3] !== props) {
29 - t1 = object?.[props.key];
30 - $[2] = object;
31 - $[3] = props;
32 - $[4] = t1;
33 - } else {
34 - t1 = $[4];
35 - }
36 - return t1;
27 + return object?.[props.key];
28 }
29
30 ```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/optional-member-expression-with-optional-member-expr-as-property.expect.md
+2 -10
@@ -14,7 +14,7 @@ function Component(props) {
14 ```javascript
15 import { unstable_useMemoCache as useMemoCache } from "react";
16 function Component(props) {
17 - const $ = useMemoCache(3);
17 + const $ = useMemoCache(1);
18 let t0;
19 if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
20 t0 = makeObject();
@@ -23,15 +23,7 @@ function Component(props) {
23 t0 = $[0];
24 }
25 const x = t0;
26 - let t1;
27 - if ($[1] !== props) {
28 - t1 = x.y?.[props.a?.[props.b?.[props.c]]];
29 - $[1] = props;
30 - $[2] = t1;
31 - } else {
32 - t1 = $[2];
33 - }
34 - return t1;
26 + return x.y?.[props.a?.[props.b?.[props.c]]];
27 }
28
29 ```
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-independently-memoized-property-load-for-method-call.expect.md
+8 -16
@@ -54,7 +54,7 @@ export const FIXTURE_ENTRYPOINT = {
54 ```javascript
55 import { unstable_useMemoCache as useMemoCache } from "react";
56 function Component(t0) {
57 - const $ = useMemoCache(11);
57 + const $ = useMemoCache(8);
58 const { label, highlightedItem } = t0;
59 const serverTime = useServerTime();
60 let t1;
@@ -63,16 +63,8 @@ function Component(t0) {
63 const highlight = new Highlight(highlightedItem);
64
65 const time = serverTime.get();
66 - let t2;
67 - if ($[5] !== time || $[6] !== label) {
68 - t2 = time / 1000 || label;
69 - $[5] = time;
70 - $[6] = label;
71 - $[7] = t2;
72 - } else {
73 - t2 = $[7];
74 - }
75 - timestampLabel = t2;
66 +
67 + timestampLabel = time / 1000 || label;
68
69 t1 = highlight.render();
70 $[0] = highlightedItem;
@@ -85,18 +77,18 @@ function Component(t0) {
77 timestampLabel = $[4];
78 }
79 let t2;
88 - if ($[8] !== t1 || $[9] !== timestampLabel) {
80 + if ($[5] !== t1 || $[6] !== timestampLabel) {
81 t2 = (
82 <>
83 {t1}
84 {timestampLabel}
85 </>
86 );
95 - $[8] = t1;
96 - $[9] = timestampLabel;
97 - $[10] = t2;
87 + $[5] = t1;
88 + $[6] = timestampLabel;
89 + $[7] = t2;
90 } else {
99 - t2 = $[10];
91 + t2 = $[7];
92 }
93 return t2;
94 }
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-unreachable-code-early-return-in-useMemo.expect.md
+18 -33
@@ -49,60 +49,45 @@ import {
49 import { ValidateMemoization, identity } from "shared-runtime";
50
51 function Component(t0) {
52 - const $ = useMemoCache(10);
52 + const $ = useMemoCache(7);
53 const { value } = t0;
54 let t1;
55 bb13: {
56 if (value == null) {
57 - if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
58 - t1 = null;
59 - break bb13;
60 - $[0] = t1;
61 - } else {
62 - t1 = $[0];
63 - }
57 + t1 = null;
58 + break bb13;
59 }
60 try {
61 let t3;
67 - if ($[1] !== value) {
62 + if ($[0] !== value) {
63 t3 = { value };
69 - $[1] = value;
70 - $[2] = t3;
71 - } else {
72 - t3 = $[2];
73 - }
74 - if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
75 - t1 = t3;
76 - $[3] = t1;
64 + $[0] = value;
65 + $[1] = t3;
66 } else {
78 - t1 = $[3];
67 + t3 = $[1];
68 }
69 + t1 = t3;
70 } catch (t2) {
81 - if ($[4] === Symbol.for("react.memo_cache_sentinel")) {
82 - t1 = null;
83 - $[4] = t1;
84 - } else {
85 - t1 = $[4];
86 - }
71 + t1 = null;
72 }
73 }
74 const result = t1;
75 let t2;
91 - if ($[5] !== value) {
76 + if ($[2] !== value) {
77 t2 = [value];
93 - $[5] = value;
94 - $[6] = t2;
78 + $[2] = value;
79 + $[3] = t2;
80 } else {
96 - t2 = $[6];
81 + t2 = $[3];
82 }
83 let t3;
99 - if ($[7] !== t2 || $[8] !== result) {
84 + if ($[4] !== t2 || $[5] !== result) {
85 t3 = <ValidateMemoization inputs={t2} output={result} />;
101 - $[7] = t2;
102 - $[8] = result;
103 - $[9] = t3;
86 + $[4] = t2;
87 + $[5] = result;
88 + $[6] = t3;
89 } else {
105 - t3 = $[9];
90 + t3 = $[6];
91 }
92 return t3;
93 }