@samitouri / QOS-React-1 / commits / 3ab47ac6bd

Replace InferReactiveIdentifiers w new inference

This PR adds one remaining feature to InferReactivePlaces: tracking indirections like LoadLocal, PropertyLoad, and similar. Consider something like: ``` // INPUT x.push(reactiveValue); // HIR t0 = LoadLocal 'x' t1 = PropertyLoad t0, 'push' t2 = LoadLocal 'reactiveValue' // reactive t3 = CallExpression mutate t0 . read t1 ( read t2 ) ``` Because a reactive value (`t2`) flows into `t0`, we want to record t0 as reactive as well. But that's just the temporary for `LoadLocal 'x'` - what's really happening is that from this point, `x` is reactive. InferReactiveIdentifiers tracked this, and now that logic is ported into InferReactivePlaces as well. That lets us remove all the actual inference from InferReactiveIdentifiers.

Joe Savona committed Nov 1, 2023 at 17:13 UTC 3ab47ac6bd2818ab04d069803cff5271012ee146
5 files changed +95 -236
compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts
+2 -1
@@ -675,8 +675,9 @@ export function printPlace(place: Place): string {
675 " ",
676 printIdentifier(place.identifier),
677 printMutableRange(place.identifier),
678 + printType(place.identifier.type),
679 + place.reactive ? "{reactive}" : null,
680 ];
679 - items.push(printType(place.identifier.type));
681 return items.filter((x) => x != null).join("");
682 }
683
compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts
+31 -3
@@ -22,7 +22,6 @@ import {
22 eachInstructionValueOperand,
23 eachTerminalOperand,
24 } from "../HIR/visitors";
25 -import { hasBackEdge } from "../Optimization/DeadCodeElimination";
25 import { assertExhaustive } from "../Utils/utils";
26
27 /**
@@ -94,9 +93,9 @@ export function inferReactivePlaces(fn: HIRFunction): void {
93 const postDominators = computePostDominatorTree(fn, {
94 includeThrowsAsExitNode: false,
95 });
97 - const hasLoop = hasBackEdge(fn);
96 const postDominatorFrontierCache = new Map<BlockId, Set<BlockId>>();
97 do {
98 + const identifierMapping = new Map<Identifier, Identifier>();
99 for (const [, block] of fn.body.blocks) {
100 for (const phi of block.phis) {
101 if (reactiveIdentifiers.isReactiveIdentifier(phi.id)) {
@@ -194,6 +193,10 @@ export function inferReactivePlaces(fn: HIRFunction): void {
193 case Effect.Store:
194 case Effect.ConditionallyMutate:
195 case Effect.Mutate: {
196 + const resolvedId = identifierMapping.get(operand.identifier);
197 + if (resolvedId !== undefined) {
198 + reactiveIdentifiers.markReactiveIdentifier(resolvedId);
199 + }
200 reactiveIdentifiers.markReactive(operand);
201 break;
202 }
@@ -219,12 +222,37 @@ export function inferReactivePlaces(fn: HIRFunction): void {
222 }
223 }
224 }
225 +
226 + switch (value.kind) {
227 + case "LoadLocal": {
228 + identifierMapping.set(
229 + instruction.lvalue.identifier,
230 + value.place.identifier
231 + );
232 + break;
233 + }
234 + case "PropertyLoad":
235 + case "ComputedLoad": {
236 + const resolvedId =
237 + identifierMapping.get(value.object.identifier) ??
238 + value.object.identifier;
239 + identifierMapping.set(instruction.lvalue.identifier, resolvedId);
240 + break;
241 + }
242 + case "LoadContext": {
243 + identifierMapping.set(
244 + instruction.lvalue.identifier,
245 + value.place.identifier
246 + );
247 + break;
248 + }
249 + }
250 }
251 for (const operand of eachTerminalOperand(block.terminal)) {
252 reactiveIdentifiers.isReactive(operand);
253 }
254 }
227 - } while (reactiveIdentifiers.snapshot() && hasLoop);
255 + } while (reactiveIdentifiers.snapshot());
256 }
257
258 /**
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CollectReactiveIdentifiers.ts new
+53
@@ -0,0 +1,53 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + */
7 +
8 +import {
9 + IdentifierId,
10 + InstructionId,
11 + Place,
12 + ReactiveFunction,
13 +} from "../HIR/HIR";
14 +import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
15 +
16 +class Visitor extends ReactiveFunctionVisitor<Set<IdentifierId>> {
17 + // Visitors don't visit lvalues as places by default, but we want to visit all places to
18 + // check for reactivity
19 + override visitLValue(
20 + id: InstructionId,
21 + lvalue: Place,
22 + state: Set<IdentifierId>
23 + ): void {
24 + this.visitPlace(id, lvalue, state);
25 + }
26 +
27 + // This visitor only infers data dependencies and does not account for control dependencies
28 + // where a variable may be assigned a different value based on some conditional, eg via two
29 + // different paths of an if statement.
30 + override visitPlace(
31 + _id: InstructionId,
32 + place: Place,
33 + state: Set<IdentifierId>
34 + ): void {
35 + if (place.reactive) {
36 + state.add(place.identifier.id);
37 + }
38 + }
39 +}
40 +
41 +/**
42 + * Computes a set of identifiers which are reactive, using the analysis previously performed
43 + * in `InferReactivePlaces`.
44 + */
45 +export function collectReactiveIdentifiers(
46 + fn: ReactiveFunction
47 +): Set<IdentifierId> {
48 + const visitor = new Visitor();
49 + const state = new Set<IdentifierId>();
50 + visitReactiveFunction(fn, visitor, state);
51 +
52 + return state;
53 +}
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveIdentifiers.ts deleted
-226
@@ -1,226 +0,0 @@
1 -/**
2 - * Copyright (c) Meta Platforms, Inc. and affiliates.
3 - *
4 - * This source code is licensed under the MIT license found in the
5 - * LICENSE file in the root directory of this source tree.
6 - */
7 -
8 -import { CompilerError } from "../CompilerError";
9 -import { Environment } from "../HIR";
10 -import {
11 - Effect,
12 - IdentifierId,
13 - InstructionId,
14 - Place,
15 - ReactiveFunction,
16 - ReactiveInstruction,
17 - getHookKind,
18 -} from "../HIR/HIR";
19 -import { eachInstructionLValue } from "../HIR/visitors";
20 -import { assertExhaustive } from "../Utils/utils";
21 -import {
22 - ReactiveFunctionVisitor,
23 - eachReactiveValueOperand,
24 - visitReactiveFunction,
25 -} from "./visitors";
26 -
27 -type IdentifierReactivity = Map<IdentifierId, boolean>;
28 -
29 -class State {
30 - env: Environment;
31 - reactivityMap: IdentifierReactivity = new Map();
32 - temporaries: Map<IdentifierId, IdentifierId> = new Map();
33 -
34 - constructor(env: Environment) {
35 - this.env = env;
36 - }
37 -}
38 -
39 -class Visitor extends ReactiveFunctionVisitor<State> {
40 - override visitLValue(
41 - _id: InstructionId,
42 - _lvalue: Place,
43 - _state: State
44 - ): void {
45 - this.visitPlace(_id, _lvalue, _state);
46 - }
47 - override visitPlace(_id: InstructionId, _place: Place, _state: State): void {
48 - if (_place.reactive) {
49 - _state.reactivityMap.set(_place.identifier.id, _place.reactive);
50 - }
51 - }
52 -
53 - override visitInstruction(instr: ReactiveInstruction, state: State): void {
54 - this.traverseInstruction(instr, state);
55 - const lval = instr.lvalue;
56 - if (lval == null) {
57 - return;
58 - }
59 - const { value } = instr;
60 - let hasReactiveInput = false;
61 - // Globals are currently treated as non-reactive this happens implicitly because LoadGlobal
62 - // has no operands which can be registered as reactive.
63 - // Consider adding an option to declare whether a given global can be reactive or not, or
64 - // a more general "treat all globals as reactive" flag.
65 - for (const operand of eachReactiveValueOperand(value)) {
66 - if (operand.effect === Effect.Store) {
67 - continue;
68 - }
69 - const ownId = operand.identifier.id;
70 - const resolvedId = state.temporaries.get(ownId);
71 - // We need to check reactivity of both the operand and its resolved source (if operand is
72 - // produced by a LoadLocal / PropertyLoad / ComputedLoad). Both the operand and its source
73 - // can have reactivity. e.g.
74 - // ```js
75 - // const o = makeObject(); // source has no reactivity
76 - // const x = o[props.x]; // x is reactive
77 - // ```
78 - if (
79 - state.reactivityMap.get(ownId) ||
80 - (resolvedId && state.reactivityMap.get(resolvedId))
81 - ) {
82 - hasReactiveInput = true;
83 - break;
84 - }
85 - }
86 - if (!hasReactiveInput) {
87 - if (
88 - instr.value.kind === "CallExpression" &&
89 - getHookKind(state.env, instr.value.callee.identifier) != null
90 - ) {
91 - // Hooks cannot be memoized. Even if they do not accept any reactive inputs,
92 - // they are not guaranteed to memoize their return value, and their result
93 - // must be assumed to be reactive.
94 - // TODO: use types or an opt-in registry of custom hook information to
95 - // allow treating safe hooks as non-reactive.
96 - hasReactiveInput = true;
97 - } else if (
98 - instr.value.kind === "MethodCall" &&
99 - getHookKind(state.env, instr.value.property.identifier) != null
100 - ) {
101 - // Same as above, but for invoking hooks via a property load such as `React.useState()`
102 - hasReactiveInput = true;
103 - }
104 - }
105 - state.reactivityMap.set(lval.identifier.id, hasReactiveInput);
106 -
107 - if (hasReactiveInput) {
108 - for (const lvalue of eachInstructionLValue(instr)) {
109 - state.reactivityMap.set(lvalue.identifier.id, true);
110 - }
111 - // all mutating effects must also be marked as reactive
112 - for (const operand of eachReactiveValueOperand(value)) {
113 - switch (operand.effect) {
114 - case Effect.Capture:
115 - case Effect.Store:
116 - case Effect.ConditionallyMutate:
117 - case Effect.Mutate: {
118 - const resolvedId: IdentifierId =
119 - state.temporaries.get(operand.identifier.id) ??
120 - operand.identifier.id;
121 - state.reactivityMap.set(resolvedId, true);
122 - break;
123 - }
124 - case Effect.Freeze:
125 - case Effect.Read: {
126 - // no-op
127 - break;
128 - }
129 - case Effect.Unknown: {
130 - CompilerError.invariant(false, {
131 - reason: "Unexpected unknown effect",
132 - description: null,
133 - loc: operand.loc,
134 - suggestions: null,
135 - });
136 - }
137 - default: {
138 - assertExhaustive(
139 - operand.effect,
140 - `Unexpected effect kind '${operand.effect}'`
141 - );
142 - }
143 - }
144 - }
145 - }
146 - if (instr.lvalue !== null) {
147 - if (instr.value.kind === "LoadLocal") {
148 - state.temporaries.set(
149 - instr.lvalue.identifier.id,
150 - instr.value.place.identifier.id
151 - );
152 - } else if (
153 - instr.value.kind === "PropertyLoad" ||
154 - instr.value.kind === "ComputedLoad"
155 - ) {
156 - const resolvedId =
157 - state.temporaries.get(instr.value.object.identifier.id) ??
158 - instr.value.object.identifier.id;
159 - state.temporaries.set(instr.lvalue.identifier.id, resolvedId);
160 - } else if (instr.value.kind === "LoadContext") {
161 - state.temporaries.set(
162 - instr.lvalue.identifier.id,
163 - instr.value.place.identifier.id
164 - );
165 - }
166 - }
167 - }
168 -}
169 -/**
170 - * Computes a map of {@link Place} -> reactivityMap. A Place is reactive if any
171 - * operant used in its construction is reactive. Sources of reactivity are
172 - * {@link ReactiveFunction.params} and HookCall return values (TODO).
173 - * Free values are currently not populated.
174 - *
175 - * This relies on alias analysis done by InferReactiveScopeVariables, which
176 - * creates reactive scopes for variables that mutate together. If one value
177 - * declared in a scope is Reactive, then the rest are marked as reactive as
178 - * well.
179 - * e.g.
180 - * ```javascript
181 - * function foo(props) {
182 - * let x = {};
183 - * let y = [];
184 - * x.y = y;
185 - * y.push(props.a)
186 - * // references to x are reactive here
187 - * }
188 - * ```
189 - * This an overestimate when two identifiers have overlapping scope, but
190 - * one is not actually reactive. However, since the same ReactiveBlock now
191 - * produces both identifiers, they are effectively both reactive (i.e.
192 - * object creation is not stable)
193 - * e.g.
194 - * ```javascript
195 - * function bar(props) {
196 - * // x and y have overlapping mutableRanges, so they share a ReactiveScope
197 - * // (even though they are not aliased together)
198 - * // technically y has no reactive inputs, but it becomes non-stable due to
199 - * // sharing a ReactiveScopeBlock with x
200 - * let x = {};
201 - * let y = [];
202 - * mutate1(x, props);
203 - * mutate2(y);
204 - * }
205 - * ```
206 - */
207 -export function inferReactiveIdentifiers(
208 - fn: ReactiveFunction
209 -): Set<IdentifierId> {
210 - const visitor = new Visitor();
211 - const state = new State(fn.env);
212 - for (const param of fn.params) {
213 - if (param.kind === "Identifier") {
214 - state.reactivityMap.set(param.identifier.id, true);
215 - } else {
216 - state.reactivityMap.set(param.place.identifier.id, true);
217 - }
218 - }
219 - visitReactiveFunction(fn, visitor, state);
220 -
221 - const result = new Set<IdentifierId>();
222 - state.reactivityMap.forEach((isReactive, id) => {
223 - if (isReactive) result.add(id);
224 - });
225 - return result;
226 -}
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts
+9 -6
@@ -11,7 +11,7 @@ import {
11 ReactiveScopeBlock,
12 isSetStateType,
13 } from "../HIR";
14 -import { inferReactiveIdentifiers } from "./InferReactiveIdentifiers";
14 +import { collectReactiveIdentifiers } from "./CollectReactiveIdentifiers";
15 import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
16
17 /**
@@ -21,14 +21,17 @@ import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
21 * This pass prunes dependencies that are guaranteed to be non-reactive.
22 */
23 export function pruneNonReactiveDependencies(fn: ReactiveFunction): void {
24 - const state = inferReactiveIdentifiers(fn);
25 - visitReactiveFunction(fn, new Visitor(), state);
24 + const reactiveIdentifiers = collectReactiveIdentifiers(fn);
25 + visitReactiveFunction(fn, new Visitor(), reactiveIdentifiers);
26 }
27
28 -type State = Set<IdentifierId>;
28 +type ReactiveIdentifiers = Set<IdentifierId>;
29
30 -class Visitor extends ReactiveFunctionVisitor<State> {
31 - override visitScope(scope: ReactiveScopeBlock, state: State): void {
30 +class Visitor extends ReactiveFunctionVisitor<ReactiveIdentifiers> {
31 + override visitScope(
32 + scope: ReactiveScopeBlock,
33 + state: ReactiveIdentifiers
34 + ): void {
35 this.traverseScope(scope, state);
36 for (const dep of scope.scope.dependencies) {
37 const isReactive =