@samitouri / QOS-React / commits / 147374d71a

[compiler] Kill markReactiveIdentifier and friends

Summary: With the previous PR we no longer need to mark identifiers as reactive in contexts where we don't have places. We already deleted most uses of markReactiveId; the last case was to track identifiers through loadlocals etc -- but we already use a disjoint alias map that accounts for loadlocals when setting reactivity. ghstack-source-id: 69ce0a78b0729da3fe9d08177bf7d827af5325fb Pull Request resolved: https://github.com/facebook/react/pull/31178

Mike Vitousek committed Oct 11, 2024 at 16:19 UTC 147374d71a73f3f63ff6532dc081d1175f783352
1 file changed +4 -42
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts
+4 -42
@@ -157,7 +157,6 @@ export function inferReactivePlaces(fn: HIRFunction): void {
157 }
158
159 do {
160 - const identifierMapping = new Map<Identifier, Identifier>();
160 for (const [, block] of fn.body.blocks) {
161 let hasReactiveControl = isReactiveControlledBlock(block.id);
162
@@ -233,10 +232,6 @@ export function inferReactivePlaces(fn: HIRFunction): void {
232 case Effect.ConditionallyMutate:
233 case Effect.Mutate: {
234 if (isMutable(instruction, operand)) {
236 - const resolvedId = identifierMapping.get(operand.identifier);
237 - if (resolvedId !== undefined) {
238 - reactiveIdentifiers.markReactiveIdentifier(resolvedId);
239 - }
235 reactiveIdentifiers.markReactive(operand);
236 }
237 break;
@@ -263,31 +258,6 @@ export function inferReactivePlaces(fn: HIRFunction): void {
258 }
259 }
260 }
266 -
267 - switch (value.kind) {
268 - case 'LoadLocal': {
269 - identifierMapping.set(
270 - instruction.lvalue.identifier,
271 - value.place.identifier,
272 - );
273 - break;
274 - }
275 - case 'PropertyLoad':
276 - case 'ComputedLoad': {
277 - const resolvedId =
278 - identifierMapping.get(value.object.identifier) ??
279 - value.object.identifier;
280 - identifierMapping.set(instruction.lvalue.identifier, resolvedId);
281 - break;
282 - }
283 - case 'LoadContext': {
284 - identifierMapping.set(
285 - instruction.lvalue.identifier,
286 - value.place.identifier,
287 - );
288 - break;
289 - }
290 - }
261 }
262 for (const operand of eachTerminalOperand(block.terminal)) {
263 reactiveIdentifiers.isReactive(operand);
@@ -372,27 +342,19 @@ class ReactivityMap {
342 }
343
344 isReactive(place: Place): boolean {
375 - const reactive = this.isReactiveIdentifier(place.identifier);
345 + const identifier =
346 + this.aliasedIdentifiers.find(place.identifier) ?? place.identifier;
347 + const reactive = this.reactive.has(identifier.id);
348 if (reactive) {
349 place.reactive = true;
350 }
351 return reactive;
352 }
353
382 - isReactiveIdentifier(inputIdentifier: Identifier): boolean {
383 - const identifier =
384 - this.aliasedIdentifiers.find(inputIdentifier) ?? inputIdentifier;
385 - return this.reactive.has(identifier.id);
386 - }
387 -
354 markReactive(place: Place): void {
355 place.reactive = true;
390 - this.markReactiveIdentifier(place.identifier);
391 - }
392 -
393 - markReactiveIdentifier(inputIdentifier: Identifier): void {
356 const identifier =
395 - this.aliasedIdentifiers.find(inputIdentifier) ?? inputIdentifier;
357 + this.aliasedIdentifiers.find(place.identifier) ?? place.identifier;
358 if (!this.reactive.has(identifier.id)) {
359 this.hasChanges = true;
360 this.reactive.add(identifier.id);