Skip to content

Commit 5addb0b

Browse files
committed
[compiler] Delete LoweredFunction.dependencies and hoisted instructions
LoweredFunction dependencies were exclusively used for dependency extraction (in `propagateScopeDeps`). Now that we have a `propagateScopeDepsHIR` that recursively traverses into nested functions, we can delete `dependencies` and their associated synthetic `LoadLocal`/`PropertyLoad` instructions. [Internal snapshot diff](https://www.internalfb.com/phabricator/paste/view/P1716950202) for this change shows ~.2% of files changed. I [read through ~60 of the changed files](https://www.internalfb.com/phabricator/paste/view/P1733074307) - most changes are due to better outlining (due to better DCE) - a few changes in memo inference are due to changed ordering ``` // source arr.map(() => contextVar.inner); // previous instructions $0 = LoadLocal arr $1 = $0.map // Below instructions are synthetic $2 = LoadLocal contextVar $3 = $2.inner $4 = Function deps=$3 context=contextVar { ... } ``` - a few changes are effectively bugfixes (see `aliased-nested-scope-fn-expr`)
1 parent e670e72 commit 5addb0b

File tree

55 files changed

+1125
-472
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+1125
-472
lines changed

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

+12-142
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import {NodePath, Scope} from '@babel/traverse';
99
import * as t from '@babel/types';
10-
import {Expression} from '@babel/types';
1110
import invariant from 'invariant';
1211
import {
1312
CompilerError,
@@ -75,7 +74,7 @@ export function lower(
7574
parent: NodePath<t.Function> | null = null,
7675
): Result<HIRFunction, CompilerError> {
7776
const builder = new HIRBuilder(env, parent ?? func, bindings, capturedRefs);
78-
const context: Array<Place> = [];
77+
const context: HIRFunction['context'] = [];
7978

8079
for (const ref of capturedRefs ?? []) {
8180
context.push({
@@ -3378,7 +3377,7 @@ function lowerFunction(
33783377
>,
33793378
): LoweredFunction | null {
33803379
const componentScope: Scope = builder.parentFunction.scope;
3381-
const captured = gatherCapturedDeps(builder, expr, componentScope);
3380+
const capturedContext = gatherCapturedContext(expr, componentScope);
33823381

33833382
/*
33843383
* TODO(gsn): In the future, we could only pass in the context identifiers
@@ -3392,7 +3391,7 @@ function lowerFunction(
33923391
expr,
33933392
builder.environment,
33943393
builder.bindings,
3395-
[...builder.context, ...captured.identifiers],
3394+
[...builder.context, ...capturedContext],
33963395
builder.parentFunction,
33973396
);
33983397
let loweredFunc: HIRFunction;
@@ -3405,7 +3404,6 @@ function lowerFunction(
34053404
loweredFunc = lowering.unwrap();
34063405
return {
34073406
func: loweredFunc,
3408-
dependencies: captured.refs,
34093407
};
34103408
}
34113409

@@ -4079,14 +4077,6 @@ function lowerAssignment(
40794077
}
40804078
}
40814079

4082-
function isValidDependency(path: NodePath<t.MemberExpression>): boolean {
4083-
const parent: NodePath<t.Node> = path.parentPath;
4084-
return (
4085-
!path.node.computed &&
4086-
!(parent.isCallExpression() && parent.get('callee') === path)
4087-
);
4088-
}
4089-
40904080
function captureScopes({from, to}: {from: Scope; to: Scope}): Set<Scope> {
40914081
let scopes: Set<Scope> = new Set();
40924082
while (from) {
@@ -4101,19 +4091,16 @@ function captureScopes({from, to}: {from: Scope; to: Scope}): Set<Scope> {
41014091
return scopes;
41024092
}
41034093

4104-
function gatherCapturedDeps(
4105-
builder: HIRBuilder,
4094+
function gatherCapturedContext(
41064095
fn: NodePath<
41074096
| t.FunctionExpression
41084097
| t.ArrowFunctionExpression
41094098
| t.FunctionDeclaration
41104099
| t.ObjectMethod
41114100
>,
41124101
componentScope: Scope,
4113-
): {identifiers: Array<t.Identifier>; refs: Array<Place>} {
4114-
const capturedIds: Map<t.Identifier, number> = new Map();
4115-
const capturedRefs: Set<Place> = new Set();
4116-
const seenPaths: Set<string> = new Set();
4102+
): Array<t.Identifier> {
4103+
const capturedIds = new Set<t.Identifier>();
41174104

41184105
/*
41194106
* Capture all the scopes from the parent of this function up to and including
@@ -4124,33 +4111,11 @@ function gatherCapturedDeps(
41244111
to: componentScope,
41254112
});
41264113

4127-
function addCapturedId(bindingIdentifier: t.Identifier): number {
4128-
if (!capturedIds.has(bindingIdentifier)) {
4129-
const index = capturedIds.size;
4130-
capturedIds.set(bindingIdentifier, index);
4131-
return index;
4132-
} else {
4133-
return capturedIds.get(bindingIdentifier)!;
4134-
}
4135-
}
4136-
41374114
function handleMaybeDependency(
4138-
path:
4139-
| NodePath<t.MemberExpression>
4140-
| NodePath<t.Identifier>
4141-
| NodePath<t.JSXOpeningElement>,
4115+
path: NodePath<t.Identifier> | NodePath<t.JSXOpeningElement>,
41424116
): void {
41434117
// Base context variable to depend on
41444118
let baseIdentifier: NodePath<t.Identifier> | NodePath<t.JSXIdentifier>;
4145-
/*
4146-
* Base expression to depend on, which (for now) may contain non side-effectful
4147-
* member expressions
4148-
*/
4149-
let dependency:
4150-
| NodePath<t.MemberExpression>
4151-
| NodePath<t.JSXMemberExpression>
4152-
| NodePath<t.Identifier>
4153-
| NodePath<t.JSXIdentifier>;
41544119
if (path.isJSXOpeningElement()) {
41554120
const name = path.get('name');
41564121
if (!(name.isJSXMemberExpression() || name.isJSXIdentifier())) {
@@ -4166,115 +4131,20 @@ function gatherCapturedDeps(
41664131
'Invalid logic in gatherCapturedDeps',
41674132
);
41684133
baseIdentifier = current;
4169-
4170-
/*
4171-
* Get the expression to depend on, which may involve PropertyLoads
4172-
* for member expressions
4173-
*/
4174-
let currentDep:
4175-
| NodePath<t.JSXMemberExpression>
4176-
| NodePath<t.Identifier>
4177-
| NodePath<t.JSXIdentifier> = baseIdentifier;
4178-
4179-
while (true) {
4180-
const nextDep: null | NodePath<t.Node> = currentDep.parentPath;
4181-
if (nextDep && nextDep.isJSXMemberExpression()) {
4182-
currentDep = nextDep;
4183-
} else {
4184-
break;
4185-
}
4186-
}
4187-
dependency = currentDep;
4188-
} else if (path.isMemberExpression()) {
4189-
// Calculate baseIdentifier
4190-
let currentId: NodePath<Expression> = path;
4191-
while (currentId.isMemberExpression()) {
4192-
currentId = currentId.get('object');
4193-
}
4194-
if (!currentId.isIdentifier()) {
4195-
return;
4196-
}
4197-
baseIdentifier = currentId;
4198-
4199-
/*
4200-
* Get the expression to depend on, which may involve PropertyLoads
4201-
* for member expressions
4202-
*/
4203-
let currentDep:
4204-
| NodePath<t.MemberExpression>
4205-
| NodePath<t.Identifier>
4206-
| NodePath<t.JSXIdentifier> = baseIdentifier;
4207-
4208-
while (true) {
4209-
const nextDep: null | NodePath<t.Node> = currentDep.parentPath;
4210-
if (
4211-
nextDep &&
4212-
nextDep.isMemberExpression() &&
4213-
isValidDependency(nextDep)
4214-
) {
4215-
currentDep = nextDep;
4216-
} else {
4217-
break;
4218-
}
4219-
}
4220-
4221-
dependency = currentDep;
42224134
} else {
42234135
baseIdentifier = path;
4224-
dependency = path;
42254136
}
42264137

42274138
/*
42284139
* Skip dependency path, as we already tried to recursively add it (+ all subexpressions)
42294140
* as a dependency.
42304141
*/
4231-
dependency.skip();
4142+
path.skip();
42324143

42334144
// Add the base identifier binding as a dependency.
42344145
const binding = baseIdentifier.scope.getBinding(baseIdentifier.node.name);
4235-
if (binding === undefined || !pureScopes.has(binding.scope)) {
4236-
return;
4237-
}
4238-
const idKey = String(addCapturedId(binding.identifier));
4239-
4240-
// Add the expression (potentially a memberexpr path) as a dependency.
4241-
let exprKey = idKey;
4242-
if (dependency.isMemberExpression()) {
4243-
let pathTokens = [];
4244-
let current: NodePath<Expression> = dependency;
4245-
while (current.isMemberExpression()) {
4246-
const property = current.get('property') as NodePath<t.Identifier>;
4247-
pathTokens.push(property.node.name);
4248-
current = current.get('object');
4249-
}
4250-
4251-
exprKey += '.' + pathTokens.reverse().join('.');
4252-
} else if (dependency.isJSXMemberExpression()) {
4253-
let pathTokens = [];
4254-
let current: NodePath<t.JSXMemberExpression | t.JSXIdentifier> =
4255-
dependency;
4256-
while (current.isJSXMemberExpression()) {
4257-
const property = current.get('property');
4258-
pathTokens.push(property.node.name);
4259-
current = current.get('object');
4260-
}
4261-
}
4262-
4263-
if (!seenPaths.has(exprKey)) {
4264-
let loweredDep: Place;
4265-
if (dependency.isJSXIdentifier()) {
4266-
loweredDep = lowerValueToTemporary(builder, {
4267-
kind: 'LoadLocal',
4268-
place: lowerIdentifier(builder, dependency),
4269-
loc: path.node.loc ?? GeneratedSource,
4270-
});
4271-
} else if (dependency.isJSXMemberExpression()) {
4272-
loweredDep = lowerJsxMemberExpression(builder, dependency);
4273-
} else {
4274-
loweredDep = lowerExpressionToTemporary(builder, dependency);
4275-
}
4276-
capturedRefs.add(loweredDep);
4277-
seenPaths.add(exprKey);
4146+
if (binding !== undefined && pureScopes.has(binding.scope)) {
4147+
capturedIds.add(binding.identifier);
42784148
}
42794149
}
42804150

@@ -4305,13 +4175,13 @@ function gatherCapturedDeps(
43054175
return;
43064176
} else if (path.isJSXElement()) {
43074177
handleMaybeDependency(path.get('openingElement'));
4308-
} else if (path.isMemberExpression() || path.isIdentifier()) {
4178+
} else if (path.isIdentifier()) {
43094179
handleMaybeDependency(path);
43104180
}
43114181
},
43124182
});
43134183

4314-
return {identifiers: [...capturedIds.keys()], refs: [...capturedRefs]};
4184+
return [...capturedIds.keys()];
43154185
}
43164186

43174187
function notNull<T>(value: T | null): value is T {

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

+1-36
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,7 @@ function collectHoistablePropertyLoadsImpl(
131131
fn: HIRFunction,
132132
context: CollectHoistablePropertyLoadsContext,
133133
): ReadonlyMap<BlockId, BlockInfo> {
134-
const functionExpressionLoads = collectFunctionExpressionFakeLoads(fn);
135-
const actuallyEvaluatedTemporaries = new Map(
136-
[...context.temporaries].filter(([id]) => !functionExpressionLoads.has(id)),
137-
);
138-
139-
const nodes = collectNonNullsInBlocks(fn, {
140-
...context,
141-
temporaries: actuallyEvaluatedTemporaries,
142-
});
134+
const nodes = collectNonNullsInBlocks(fn, context);
143135
propagateNonNull(fn, nodes, context.registry);
144136

145137
if (DEBUG_PRINT) {
@@ -598,30 +590,3 @@ function reduceMaybeOptionalChains(
598590
}
599591
} while (changed);
600592
}
601-
602-
function collectFunctionExpressionFakeLoads(
603-
fn: HIRFunction,
604-
): Set<IdentifierId> {
605-
const sources = new Map<IdentifierId, IdentifierId>();
606-
const functionExpressionReferences = new Set<IdentifierId>();
607-
608-
for (const [_, block] of fn.body.blocks) {
609-
for (const {lvalue, value} of block.instructions) {
610-
if (
611-
value.kind === 'FunctionExpression' ||
612-
value.kind === 'ObjectMethod'
613-
) {
614-
for (const reference of value.loweredFunc.dependencies) {
615-
let curr: IdentifierId | undefined = reference.identifier.id;
616-
while (curr != null) {
617-
functionExpressionReferences.add(curr);
618-
curr = sources.get(curr);
619-
}
620-
}
621-
} else if (value.kind === 'PropertyLoad') {
622-
sources.set(lvalue.identifier.id, value.object.identifier.id);
623-
}
624-
}
625-
}
626-
return functionExpressionReferences;
627-
}

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

-2
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,6 @@ const EnvironmentConfigSchema = z.object({
245245
*/
246246
enableUseTypeAnnotations: z.boolean().default(false),
247247

248-
enableFunctionDependencyRewrite: z.boolean().default(true),
249-
250248
/**
251249
* Enables inference of optional dependency chains. Without this flag
252250
* a property chain such as `props?.items?.foo` will infer as a dep on

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

-1
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,6 @@ export type ObjectProperty = {
722722
};
723723

724724
export type LoweredFunction = {
725-
dependencies: Array<Place>;
726725
func: HIRFunction;
727726
};
728727

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

+16-1
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,14 @@ function collectScopeInfo(fn: HIRFunction): ScopeInfo {
148148
const scope = place.identifier.scope;
149149
if (scope != null) {
150150
placeScopes.set(place, scope);
151+
/**
152+
* Record both mutating and non-mutating scopes to merge scopes with
153+
* still-mutating values with inner scopes that alias those values
154+
* (see `nonmutating-capture-in-unsplittable-memo-block`)
155+
*
156+
* Note that this isn't perfect, as it also leads to merging of mutating
157+
* scopes with JSX single-instruction scopes (see `mutation-within-jsx`)
158+
*/
151159
if (scope.range.start !== scope.range.end) {
152160
getOrInsertDefault(scopeStarts, scope.range.start, new Set()).add(
153161
scope,
@@ -254,7 +262,7 @@ function visitPlace(
254262
* of the stack to the mutated outer scope.
255263
*/
256264
const placeScope = getPlaceScope(id, place);
257-
if (placeScope != null && isMutable({id} as any, place)) {
265+
if (placeScope != null && isMutable({id}, place)) {
258266
const placeScopeIdx = activeScopes.indexOf(placeScope);
259267
if (placeScopeIdx !== -1 && placeScopeIdx !== activeScopes.length - 1) {
260268
joined.union([placeScope, ...activeScopes.slice(placeScopeIdx + 1)]);
@@ -275,6 +283,13 @@ function getOverlappingReactiveScopes(
275283
for (const instr of block.instructions) {
276284
visitInstructionId(instr.id, context, state);
277285
for (const place of eachInstructionOperand(instr)) {
286+
if (
287+
(instr.value.kind === 'FunctionExpression' ||
288+
instr.value.kind === 'ObjectMethod') &&
289+
place.identifier.type.kind === 'Primitive'
290+
) {
291+
continue;
292+
}
278293
visitPlace(instr.id, place, state);
279294
}
280295
for (const place of eachInstructionLValue(instr)) {

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

+1-4
Original file line numberDiff line numberDiff line change
@@ -538,9 +538,6 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
538538
.split('\n')
539539
.map(line => ` ${line}`)
540540
.join('\n');
541-
const deps = instrValue.loweredFunc.dependencies
542-
.map(dep => printPlace(dep))
543-
.join(',');
544541
const context = instrValue.loweredFunc.func.context
545542
.map(dep => printPlace(dep))
546543
.join(',');
@@ -557,7 +554,7 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
557554
})
558555
.join(', ') ?? '';
559556
const type = printType(instrValue.loweredFunc.func.returnType).trim();
560-
value = `${kind} ${name} @deps[${deps}] @context[${context}] @effects[${effects}]${type !== '' ? ` return${type}` : ''}:\n${fn}`;
557+
value = `${kind} ${name} @context[${context}] @effects[${effects}]${type !== '' ? ` return${type}` : ''}:\n${fn}`;
561558
break;
562559
}
563560
case 'TaggedTemplateExpression': {

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -738,9 +738,8 @@ function collectDependencies(
738738
}
739739
for (const instr of block.instructions) {
740740
if (
741-
fn.env.config.enableFunctionDependencyRewrite &&
742-
(instr.value.kind === 'FunctionExpression' ||
743-
instr.value.kind === 'ObjectMethod')
741+
instr.value.kind === 'FunctionExpression' ||
742+
instr.value.kind === 'ObjectMethod'
744743
) {
745744
context.declare(instr.lvalue.identifier, {
746745
id: instr.id,

0 commit comments

Comments
 (0)