Skip to content

Commit 31f44f8

Browse files
committed
Update base for rebase on "[compiler] Optimize emission in normal (non-value) blocks"
In #29863 I tried to find a clean way to share code for emitting instructions between value blocks and regular blocks. The catch is that value blocks have special meaning for their final instruction — that's the value of the block — so reordering can't change the last instruction. However, in finding a clean way to share code for these two categories of code, i also inadvertently reduced the effectiveness of the optimization. This PR updates to use different strategies for these two kinds of blocks: value blocks use the code from #29863 where we first emit all non-reorderable instructions in their original order, _then_ try to emit reorderable values. The reason this is suboptimal, though, is that we want to move instructions closer to their dependencies so that they can invalidate (merge) together. Emitting the reorderable values last prevents this. So for normal blocks, we now emit terminal operands first. This will invariably cause _some_ of the non-reorderable instructions to be emitted, but it will intersperse reoderable instructions in between, right after their dependencies. This maximizes our ability to merge scopes. I think the complexity cost of two strategies is worth the benefit, though i still need to double-check all the output changes. [ghstack-poisoned]
2 parents cf0d02b + 0f56841 commit 31f44f8

File tree

76 files changed

+1794
-578
lines changed

Some content is hidden

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

76 files changed

+1794
-578
lines changed

.eslintrc.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,11 @@ module.exports = {
484484
},
485485
},
486486
{
487-
files: ['packages/react-devtools-extensions/**/*.js'],
487+
files: [
488+
'packages/react-devtools-extensions/**/*.js',
489+
'packages/react-devtools-shared/src/hook.js',
490+
'packages/react-devtools-shared/src/backend/console.js',
491+
],
488492
globals: {
489493
__IS_CHROME__: 'readonly',
490494
__IS_FIREFOX__: 'readonly',

.prettierignore

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
build
22

3-
compiler
43
packages/react-devtools-core/dist
54
packages/react-devtools-extensions/chrome/build
65
packages/react-devtools-extensions/firefox/build

compiler/apps/playground/components/Editor/Output.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ async function tabify(source: string, compilerOutput: CompilerOutput) {
104104
passName,
105105
<TextTabContent
106106
output={text}
107-
diff={lastPassOutput ?? null}
107+
diff={passName !== "HIR" ? lastPassOutput : null}
108108
showInfoPanel={true}
109109
></TextTabContent>
110110
);
@@ -195,7 +195,7 @@ function Output({ store, compilerOutput }: Props) {
195195
if (result.kind === "hir" || result.kind === "reactive") {
196196
currResult += `function ${result.fnName}\n\n${result.value}`;
197197
}
198-
if (currResult !== lastResult) {
198+
if (passName !== "HIR" && currResult !== lastResult) {
199199
changedPasses.add(passName);
200200
}
201201
lastResult = currResult;

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ function* runWithEnvironment(
254254
memoizeFbtOperandsInSameScope(hir);
255255
yield log({
256256
kind: "hir",
257-
name: "MemoizeFbtOperandsInSameScope",
257+
name: "MemoizeFbtAndMacroOperandsInSameScope",
258258
value: hir,
259259
});
260260

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

+17
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,23 @@ const EnvironmentConfigSchema = z.object({
407407
* and identifiers have been changed.
408408
*/
409409
hookPattern: z.string().nullable().default(null),
410+
411+
/**
412+
* If enabled, this will treat objects named as `ref` or if their names end with the substring `Ref`,
413+
* and contain a property named `current`, as React refs.
414+
*
415+
* ```
416+
* const ref = useMyRef();
417+
* const myRef = useMyRef2();
418+
* useEffect(() => {
419+
* ref.current = ...;
420+
* myRef.current = ...;
421+
* })
422+
* ```
423+
*
424+
* Here the variables `ref` and `myRef` will be typed as Refs.
425+
*/
426+
enableTreatRefLikeIdentifiersAsRefs: z.boolean().nullable().default(false),
410427
});
411428

412429
export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;

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

+13
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Effect, ValueKind, ValueReason } from "./HIR";
99
import {
1010
BUILTIN_SHAPES,
1111
BuiltInArrayId,
12+
BuiltInUseActionStateId,
1213
BuiltInUseEffectHookId,
1314
BuiltInUseInsertionEffectHookId,
1415
BuiltInUseLayoutEffectHookId,
@@ -266,6 +267,18 @@ const REACT_APIS: Array<[string, BuiltInType]> = [
266267
returnValueReason: ValueReason.State,
267268
}),
268269
],
270+
[
271+
"useActionState",
272+
addHook(DEFAULT_SHAPES, {
273+
positionalParams: [],
274+
restParam: Effect.Freeze,
275+
returnType: { kind: "Object", shapeId: BuiltInUseActionStateId },
276+
calleeEffect: Effect.Read,
277+
hookKind: "useActionState",
278+
returnValueKind: ValueKind.Frozen,
279+
returnValueReason: ValueReason.State,
280+
}),
281+
],
269282
[
270283
"useReducer",
271284
addHook(DEFAULT_SHAPES, {

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

+16
Original file line numberDiff line numberDiff line change
@@ -1543,6 +1543,18 @@ export function isSetStateType(id: Identifier): boolean {
15431543
return id.type.kind === "Function" && id.type.shapeId === "BuiltInSetState";
15441544
}
15451545

1546+
export function isUseActionStateType(id: Identifier): boolean {
1547+
return (
1548+
id.type.kind === "Object" && id.type.shapeId === "BuiltInUseActionState"
1549+
);
1550+
}
1551+
1552+
export function isSetActionStateType(id: Identifier): boolean {
1553+
return (
1554+
id.type.kind === "Function" && id.type.shapeId === "BuiltInSetActionState"
1555+
);
1556+
}
1557+
15461558
export function isUseReducerType(id: Identifier): boolean {
15471559
return id.type.kind === "Function" && id.type.shapeId === "BuiltInUseReducer";
15481560
}
@@ -1551,6 +1563,10 @@ export function isDispatcherType(id: Identifier): boolean {
15511563
return id.type.kind === "Function" && id.type.shapeId === "BuiltInDispatch";
15521564
}
15531565

1566+
export function isStableType(id: Identifier): boolean {
1567+
return isSetStateType(id) || isSetActionStateType(id) || isDispatcherType(id);
1568+
}
1569+
15541570
export function isUseEffectHookType(id: Identifier): boolean {
15551571
return (
15561572
id.type.kind === "Function" && id.type.shapeId === "BuiltInUseEffectHook"

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

+22
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ function addShape(
118118
export type HookKind =
119119
| "useContext"
120120
| "useState"
121+
| "useActionState"
121122
| "useReducer"
122123
| "useRef"
123124
| "useEffect"
@@ -195,6 +196,8 @@ export const BuiltInJsxId = "BuiltInJsx";
195196
export const BuiltInObjectId = "BuiltInObject";
196197
export const BuiltInUseStateId = "BuiltInUseState";
197198
export const BuiltInSetStateId = "BuiltInSetState";
199+
export const BuiltInUseActionStateId = "BuiltInUseActionState";
200+
export const BuiltInSetActionStateId = "BuiltInSetActionState";
198201
export const BuiltInUseRefId = "BuiltInUseRefId";
199202
export const BuiltInRefValueId = "BuiltInRefValue";
200203
export const BuiltInMixedReadonlyId = "BuiltInMixedReadonly";
@@ -396,6 +399,25 @@ addObject(BUILTIN_SHAPES, BuiltInUseStateId, [
396399
],
397400
]);
398401

402+
addObject(BUILTIN_SHAPES, BuiltInUseActionStateId, [
403+
["0", { kind: "Poly" }],
404+
[
405+
"1",
406+
addFunction(
407+
BUILTIN_SHAPES,
408+
[],
409+
{
410+
positionalParams: [],
411+
restParam: Effect.Freeze,
412+
returnType: PRIMITIVE_TYPE,
413+
calleeEffect: Effect.Read,
414+
returnValueKind: ValueKind.Primitive,
415+
},
416+
BuiltInSetActionStateId
417+
),
418+
],
419+
]);
420+
399421
addObject(BUILTIN_SHAPES, BuiltInUseReducerId, [
400422
["0", { kind: "Poly" }],
401423
[

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

+8-4
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ export type PhiType = {
5757
};
5858
export type PropType = {
5959
kind: "Property";
60-
object: Type;
60+
objectType: Type;
61+
objectName: string;
6162
propertyName: string;
6263
};
6364

@@ -124,7 +125,8 @@ export function duplicateType(type: Type): Type {
124125
case "Property": {
125126
return {
126127
kind: "Property",
127-
object: duplicateType(type.object),
128+
objectType: duplicateType(type.objectType),
129+
objectName: type.objectName,
128130
propertyName: type.propertyName,
129131
};
130132
}
@@ -165,11 +167,13 @@ function objectMethodTypeEquals(tA: Type, tB: Type): boolean {
165167

166168
function propTypeEquals(tA: Type, tB: Type): boolean {
167169
if (tA.kind === "Property" && tB.kind === "Property") {
168-
if (!typeEquals(tA.object, tB.object)) {
170+
if (!typeEquals(tA.objectType, tB.objectType)) {
169171
return false;
170172
}
171173

172-
return tA.propertyName === tB.propertyName;
174+
return (
175+
tA.propertyName === tB.propertyName && tA.objectName === tB.objectName
176+
);
173177
}
174178

175179
return false;

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

+2-6
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ import {
1515
Place,
1616
computePostDominatorTree,
1717
getHookKind,
18-
isDispatcherType,
19-
isSetStateType,
18+
isStableType,
2019
isUseOperator,
2120
} from "../HIR";
2221
import { PostDominator } from "../HIR/Dominator";
@@ -220,10 +219,7 @@ export function inferReactivePlaces(fn: HIRFunction): void {
220219

221220
if (hasReactiveInput) {
222221
for (const lvalue of eachInstructionLValue(instruction)) {
223-
if (
224-
isSetStateType(lvalue.identifier) ||
225-
isDispatcherType(lvalue.identifier)
226-
) {
222+
if (isStableType(lvalue.identifier)) {
227223
continue;
228224
}
229225
reactiveIdentifiers.markReactive(lvalue);

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
import { eachReactiveValueOperand } from "./visitors";
1616

1717
/**
18-
* This pass supports the
1918
* This pass supports the `fbt` translation system (https://facebook.github.io/fbt/)
2019
* as well as similar user-configurable macro-like APIs where it's important that
2120
* the name of the function not be changed, and it's literal arguments not be turned
@@ -75,7 +74,7 @@ function visit(
7574
for (const instruction of block.instructions) {
7675
const { lvalue, value } = instruction;
7776
if (lvalue === null) {
78-
return;
77+
continue;
7978
}
8079
if (
8180
value.kind === "Primitive" &&
@@ -96,7 +95,7 @@ function visit(
9695
} else if (isFbtCallExpression(fbtValues, value)) {
9796
const fbtScope = lvalue.identifier.scope;
9897
if (fbtScope === null) {
99-
return;
98+
continue;
10099
}
101100

102101
/*
@@ -122,7 +121,7 @@ function visit(
122121
) {
123122
const fbtScope = lvalue.identifier.scope;
124123
if (fbtScope === null) {
125-
return;
124+
continue;
126125
}
127126

128127
/*

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonReactiveDependencies.ts

+3-7
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import {
1010
ReactiveFunction,
1111
ReactiveInstruction,
1212
ReactiveScopeBlock,
13-
isDispatcherType,
14-
isSetStateType,
13+
isStableType,
1514
} from "../HIR";
1615
import { eachPatternOperand } from "../HIR/visitors";
1716
import { collectReactiveIdentifiers } from "./CollectReactiveIdentifiers";
@@ -57,10 +56,7 @@ class Visitor extends ReactiveFunctionVisitor<ReactiveIdentifiers> {
5756
case "Destructure": {
5857
if (state.has(value.value.identifier.id)) {
5958
for (const lvalue of eachPatternOperand(value.lvalue.pattern)) {
60-
if (
61-
isSetStateType(lvalue.identifier) ||
62-
isDispatcherType(lvalue.identifier)
63-
) {
59+
if (isStableType(lvalue.identifier)) {
6460
continue;
6561
}
6662
state.add(lvalue.identifier.id);
@@ -75,7 +71,7 @@ class Visitor extends ReactiveFunctionVisitor<ReactiveIdentifiers> {
7571
if (
7672
lvalue !== null &&
7773
state.has(value.object.identifier.id) &&
78-
!isSetStateType(lvalue.identifier)
74+
!isStableType(lvalue.identifier)
7975
) {
8076
state.add(lvalue.identifier.id);
8177
}

0 commit comments

Comments
 (0)