Skip to content

Commit 94cf60b

Browse files
authored
[compiler] New inference repros/fixes (#33584)
Substantially improves the last major known issue with the new inference model's implementation: inferring effects of function expressions. I knowingly used a really simple (dumb) approach in InferFunctionExpressionAliasingEffects but it worked surprisingly well on a ton of code. However, investigating during the sync I saw that we the algorithm was literally running out of memory, or crashing from arrays that exceeded the maximum capacity. We were accumluating data flow in a way that could lead to lists of data flow captures compounding on themselves and growing very large very quickly. Plus, we were incorrectly recording some data flow, leading to cases where we reported false positive "can't mutate frozen value" for example. So I went back to the drawing board. InferMutationAliasingRanges already builds up a data flow graph which it uses to figure out what values would be affected by mutations of other values, and update mutable ranges. Well, the key question that we really want to answer for inferring a function expression's aliasing effects is which values alias/capture where. Per the docs I wrote up, we only have to record such aliasing _if they are observable via mutations_. So, lightbulb: simulate mutations of the params, free variables, and return of the function expression and see which params/free-vars would be affected! That's what we do now, giving us precise information about which such values alias/capture where. When the "into" is a param/context-var we use Capture, iwhen the destination is the return we use Alias to be conservative. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33584). * #33626 * #33625 * #33624 * __->__ #33584
1 parent bbc13fa commit 94cf60b

18 files changed

+746
-265
lines changed

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1770,6 +1770,10 @@ export function isUseStateType(id: Identifier): boolean {
17701770
return id.type.kind === 'Object' && id.type.shapeId === 'BuiltInUseState';
17711771
}
17721772

1773+
export function isJsxType(type: Type): boolean {
1774+
return type.kind === 'Object' && type.shapeId === 'BuiltInJsx';
1775+
}
1776+
17731777
export function isRefOrRefValue(id: Identifier): boolean {
17741778
return isUseRefType(id) || isRefValueType(id);
17751779
}

compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,9 @@ import {inferReactiveScopeVariables} from '../ReactiveScopes';
2020
import {rewriteInstructionKindsBasedOnReassignment} from '../SSA';
2121
import {inferMutableRanges} from './InferMutableRanges';
2222
import inferReferenceEffects from './InferReferenceEffects';
23-
import {assertExhaustive, retainWhere} from '../Utils/utils';
23+
import {assertExhaustive} from '../Utils/utils';
2424
import {inferMutationAliasingEffects} from './InferMutationAliasingEffects';
25-
import {inferFunctionExpressionAliasingEffectsSignature} from './InferFunctionExpressionAliasingEffectsSignature';
2625
import {inferMutationAliasingRanges} from './InferMutationAliasingRanges';
27-
import {hashEffect} from './AliasingEffects';
2826

2927
export default function analyseFunctions(func: HIRFunction): void {
3028
for (const [_, block] of func.body.blocks) {
@@ -69,38 +67,20 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
6967
analyseFunctions(fn);
7068
inferMutationAliasingEffects(fn, {isFunctionExpression: true});
7169
deadCodeElimination(fn);
72-
inferMutationAliasingRanges(fn, {isFunctionExpression: true});
70+
const functionEffects = inferMutationAliasingRanges(fn, {
71+
isFunctionExpression: true,
72+
}).unwrap();
7373
rewriteInstructionKindsBasedOnReassignment(fn);
7474
inferReactiveScopeVariables(fn);
75-
const effects = inferFunctionExpressionAliasingEffectsSignature(fn);
76-
fn.env.logger?.debugLogIRs?.({
77-
kind: 'hir',
78-
name: 'AnalyseFunction (inner)',
79-
value: fn,
80-
});
81-
if (effects != null) {
82-
fn.aliasingEffects ??= [];
83-
fn.aliasingEffects?.push(...effects);
84-
}
85-
if (fn.aliasingEffects != null) {
86-
const seen = new Set<string>();
87-
retainWhere(fn.aliasingEffects, effect => {
88-
const hash = hashEffect(effect);
89-
if (seen.has(hash)) {
90-
return false;
91-
}
92-
seen.add(hash);
93-
return true;
94-
});
95-
}
75+
fn.aliasingEffects = functionEffects;
9676

9777
/**
9878
* Phase 2: populate the Effect of each context variable to use in inferring
9979
* the outer function. For example, InferMutationAliasingEffects uses context variable
10080
* effects to decide if the function may be mutable or not.
10181
*/
10282
const capturedOrMutated = new Set<IdentifierId>();
103-
for (const effect of effects ?? []) {
83+
for (const effect of functionEffects) {
10484
switch (effect.kind) {
10585
case 'Assign':
10686
case 'Alias':
@@ -152,6 +132,12 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
152132
operand.effect = Effect.Read;
153133
}
154134
}
135+
136+
fn.env.logger?.debugLogIRs?.({
137+
kind: 'hir',
138+
name: 'AnalyseFunction (inner)',
139+
value: fn,
140+
});
155141
}
156142

157143
function lower(func: HIRFunction): void {

compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionExpressionAliasingEffectsSignature.ts

Lines changed: 0 additions & 206 deletions
This file was deleted.

compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,8 @@ function applyEffect(
822822
const functionValues = state.values(effect.function);
823823
if (
824824
functionValues.length === 1 &&
825-
functionValues[0].kind === 'FunctionExpression'
825+
functionValues[0].kind === 'FunctionExpression' &&
826+
functionValues[0].loweredFunc.func.aliasingEffects != null
826827
) {
827828
/*
828829
* We're calling a locally declared function, we already know it's effects!
@@ -2126,8 +2127,6 @@ function computeEffectsForLegacySignature(
21262127
const mutateIterator = conditionallyMutateIterator(place);
21272128
if (mutateIterator != null) {
21282129
effects.push(mutateIterator);
2129-
// TODO: should we always push to captures?
2130-
captures.push(place);
21312130
}
21322131
effects.push({
21332132
kind: 'Capture',

0 commit comments

Comments
 (0)