From bf37b915bd615b583222ae0991d8c858de5351c1 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 17 Oct 2025 10:50:17 -0700 Subject: [PATCH] [compiler] Optimize props spread for common cases As part of the new inference model we updated to (correctly) treat destructuring spread as creating a new mutable object. This had the unfortunate side-effect of reducing precision on destructuring of props, though: ```js function Component({x, ...rest}) { const z = rest.z; identity(z); return ; } ``` Memoized as the following, where we don't realize that `z` is actually frozen: ```js function Component(t0) { const $ = _c(6); let x; let z; if ($[0] !== t0) { const { x: t1, ...rest } = t0; x = t1; z = rest.z; identity(z); ... ``` #34341 was our first thought of how to do this (thanks @poteto for exploring this idea!). But during review it became clear that it was a bit more complicated than I had thought. So this PR explores a more conservative alternative. The idea is: * Track known sources of frozen values: component props, hook params, and hook return values. * Find all object spreads where the rvalue is a known frozen value. * Look at how such objects are used, and if they are only used to access properties (PropertyLoad/Destructure), pass to hooks, or pass to jsx then we can be very confident the object is not mutated. We consider any such objects to be frozen, even though technically spread creates a new object. See new fixtures for more examples. --- .../Inference/InferMutationAliasingEffects.ts | 164 +++++++++++++++++- .../nonmutated-spread-hook-return.expect.md | 63 +++++++ .../compiler/nonmutated-spread-hook-return.js | 13 ++ .../nonmutated-spread-props-jsx.expect.md | 57 ++++++ .../compiler/nonmutated-spread-props-jsx.js | 10 ++ ...d-spread-props-local-indirection.expect.md | 63 +++++++ ...nmutated-spread-props-local-indirection.js | 13 ++ .../nonmutated-spread-props.expect.md | 61 +++++++ .../compiler/nonmutated-spread-props.js | 12 ++ 9 files changed, 455 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index d2f2ba61d6..94be2100f5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -19,6 +19,7 @@ import { Environment, FunctionExpression, GeneratedSource, + getHookKind, HIRFunction, Hole, IdentifierId, @@ -198,6 +199,7 @@ export function inferMutationAliasingEffects( isFunctionExpression, fn, hoistedContextDeclarations, + findNonMutatedDestructureSpreads(fn), ); let iterationCount = 0; @@ -287,15 +289,18 @@ class Context { isFuctionExpression: boolean; fn: HIRFunction; hoistedContextDeclarations: Map; + nonMutatingSpreads: Set; constructor( isFunctionExpression: boolean, fn: HIRFunction, hoistedContextDeclarations: Map, + nonMutatingSpreads: Set, ) { this.isFuctionExpression = isFunctionExpression; this.fn = fn; this.hoistedContextDeclarations = hoistedContextDeclarations; + this.nonMutatingSpreads = nonMutatingSpreads; } cacheApplySignature( @@ -322,6 +327,161 @@ class Context { } } +/** + * Finds objects created via ObjectPattern spread destructuring + * (`const {x, ...spread} = ...`) where a) the rvalue is known frozen and + * b) the spread value cannot possibly be directly mutated. The idea is that + * for this set of values, we can treat the spread object as frozen. + * + * The primary use case for this is props spreading: + * + * ``` + * function Component({prop, ...otherProps}) { + * const transformedProp = transform(prop, otherProps.foo); + * // pass `otherProps` down: + * return ; + * } + * ``` + * + * Here we know that since `otherProps` cannot be mutated, we don't have to treat + * it as mutable: `otherProps.foo` only reads a value that must be frozen, so it + * can be treated as frozen too. + */ +function findNonMutatedDestructureSpreads(fn: HIRFunction): Set { + const knownFrozen = new Set(); + if (fn.fnType === 'Component') { + const [props] = fn.params; + if (props != null && props.kind === 'Identifier') { + knownFrozen.add(props.identifier.id); + } + } else { + for (const param of fn.params) { + if (param.kind === 'Identifier') { + knownFrozen.add(param.identifier.id); + } + } + } + + // Map of temporaries to identifiers for spread objects + const candidateNonMutatingSpreads = new Map(); + for (const block of fn.body.blocks.values()) { + if (candidateNonMutatingSpreads.size !== 0) { + for (const phi of block.phis) { + for (const operand of phi.operands.values()) { + const spread = candidateNonMutatingSpreads.get(operand.identifier.id); + if (spread != null) { + candidateNonMutatingSpreads.delete(spread); + } + } + } + } + for (const instr of block.instructions) { + const {lvalue, value} = instr; + switch (value.kind) { + case 'Destructure': { + if ( + !knownFrozen.has(value.value.identifier.id) || + !( + value.lvalue.kind === InstructionKind.Let || + value.lvalue.kind === InstructionKind.Const + ) || + value.lvalue.pattern.kind !== 'ObjectPattern' + ) { + continue; + } + for (const item of value.lvalue.pattern.properties) { + if (item.kind !== 'Spread') { + continue; + } + candidateNonMutatingSpreads.set( + item.place.identifier.id, + item.place.identifier.id, + ); + } + break; + } + case 'LoadLocal': { + const spread = candidateNonMutatingSpreads.get( + value.place.identifier.id, + ); + if (spread != null) { + candidateNonMutatingSpreads.set(lvalue.identifier.id, spread); + } + break; + } + case 'StoreLocal': { + const spread = candidateNonMutatingSpreads.get( + value.value.identifier.id, + ); + if (spread != null) { + candidateNonMutatingSpreads.set(lvalue.identifier.id, spread); + candidateNonMutatingSpreads.set( + value.lvalue.place.identifier.id, + spread, + ); + } + break; + } + case 'JsxFragment': + case 'JsxExpression': { + // Passing objects created with spread to jsx can't mutate them + break; + } + case 'PropertyLoad': { + // Properties must be frozen since the original value was frozen + break; + } + case 'CallExpression': + case 'MethodCall': { + const callee = + value.kind === 'CallExpression' ? value.callee : value.property; + if (getHookKind(fn.env, callee.identifier) != null) { + // Hook calls have frozen arguments, and non-ref returns are frozen + if (!isRefOrRefValue(lvalue.identifier)) { + knownFrozen.add(lvalue.identifier.id); + } + } else { + // Non-hook calls check their operands, since they are potentially mutable + if (candidateNonMutatingSpreads.size !== 0) { + // Otherwise any reference to the spread object itself may mutate + for (const operand of eachInstructionValueOperand(value)) { + const spread = candidateNonMutatingSpreads.get( + operand.identifier.id, + ); + if (spread != null) { + candidateNonMutatingSpreads.delete(spread); + } + } + } + } + break; + } + default: { + if (candidateNonMutatingSpreads.size !== 0) { + // Otherwise any reference to the spread object itself may mutate + for (const operand of eachInstructionValueOperand(value)) { + const spread = candidateNonMutatingSpreads.get( + operand.identifier.id, + ); + if (spread != null) { + candidateNonMutatingSpreads.delete(spread); + } + } + } + } + } + } + } + + const nonMutatingSpreads = new Set(); + for (const [key, value] of candidateNonMutatingSpreads) { + if (key === value) { + nonMutatingSpreads.add(key); + } + } + return nonMutatingSpreads; +} + function inferParam( param: Place | SpreadPattern, initialState: InferenceState, @@ -2054,7 +2214,9 @@ function computeSignatureForInstruction( kind: 'Create', into: place, reason: ValueReason.Other, - value: ValueKind.Mutable, + value: context.nonMutatingSpreads.has(place.identifier.id) + ? ValueKind.Frozen + : ValueKind.Mutable, }); effects.push({ kind: 'Capture', diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.expect.md new file mode 100644 index 0000000000..9a4f0c179f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +import {identity, Stringify, useIdentity} from 'shared-runtime'; + +function Component(props) { + const {x, ...rest} = useIdentity(props); + const z = rest.z; + identity(z); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity, Stringify, useIdentity } from "shared-runtime"; + +function Component(props) { + const $ = _c(6); + const t0 = useIdentity(props); + let rest; + let x; + if ($[0] !== t0) { + ({ x, ...rest } = t0); + $[0] = t0; + $[1] = rest; + $[2] = x; + } else { + rest = $[1]; + x = $[2]; + } + const z = rest.z; + identity(z); + let t1; + if ($[3] !== x || $[4] !== z) { + t1 = ; + $[3] = x; + $[4] = z; + $[5] = t1; + } else { + t1 = $[5]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ x: "Hello", z: "World" }], +}; + +``` + +### Eval output +(kind: ok)
{"x":"Hello","z":"World"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.js new file mode 100644 index 0000000000..c4447f7be6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-hook-return.js @@ -0,0 +1,13 @@ +import {identity, Stringify, useIdentity} from 'shared-runtime'; + +function Component(props) { + const {x, ...rest} = useIdentity(props); + const z = rest.z; + identity(z); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.expect.md new file mode 100644 index 0000000000..5335705c5d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.expect.md @@ -0,0 +1,57 @@ + +## Input + +```javascript +import {identity, Stringify} from 'shared-runtime'; + +function Component({x, ...rest}) { + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity, Stringify } from "shared-runtime"; + +function Component(t0) { + const $ = _c(6); + let rest; + let x; + if ($[0] !== t0) { + ({ x, ...rest } = t0); + $[0] = t0; + $[1] = rest; + $[2] = x; + } else { + rest = $[1]; + x = $[2]; + } + let t1; + if ($[3] !== rest || $[4] !== x) { + t1 = ; + $[3] = rest; + $[4] = x; + $[5] = t1; + } else { + t1 = $[5]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ x: "Hello", z: "World" }], +}; + +``` + +### Eval output +(kind: ok)
{"z":"World","x":"Hello"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.js new file mode 100644 index 0000000000..d9f24264d6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-jsx.js @@ -0,0 +1,10 @@ +import {identity, Stringify} from 'shared-runtime'; + +function Component({x, ...rest}) { + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.expect.md new file mode 100644 index 0000000000..7a435adcaa --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +import {identity, Stringify} from 'shared-runtime'; + +function Component({x, ...rest}) { + const restAlias = rest; + const z = restAlias.z; + identity(z); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity, Stringify } from "shared-runtime"; + +function Component(t0) { + const $ = _c(6); + let rest; + let x; + if ($[0] !== t0) { + ({ x, ...rest } = t0); + $[0] = t0; + $[1] = rest; + $[2] = x; + } else { + rest = $[1]; + x = $[2]; + } + const restAlias = rest; + const z = restAlias.z; + identity(z); + let t1; + if ($[3] !== x || $[4] !== z) { + t1 = ; + $[3] = x; + $[4] = z; + $[5] = t1; + } else { + t1 = $[5]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ x: "Hello", z: "World" }], +}; + +``` + +### Eval output +(kind: ok)
{"x":"Hello","z":"World"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.js new file mode 100644 index 0000000000..b1d26ab7f7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props-local-indirection.js @@ -0,0 +1,13 @@ +import {identity, Stringify} from 'shared-runtime'; + +function Component({x, ...rest}) { + const restAlias = rest; + const z = restAlias.z; + identity(z); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.expect.md new file mode 100644 index 0000000000..a8b1c2d747 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.expect.md @@ -0,0 +1,61 @@ + +## Input + +```javascript +import {identity, Stringify} from 'shared-runtime'; + +function Component({x, ...rest}) { + const z = rest.z; + identity(z); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity, Stringify } from "shared-runtime"; + +function Component(t0) { + const $ = _c(6); + let rest; + let x; + if ($[0] !== t0) { + ({ x, ...rest } = t0); + $[0] = t0; + $[1] = rest; + $[2] = x; + } else { + rest = $[1]; + x = $[2]; + } + const z = rest.z; + identity(z); + let t1; + if ($[3] !== x || $[4] !== z) { + t1 = ; + $[3] = x; + $[4] = z; + $[5] = t1; + } else { + t1 = $[5]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ x: "Hello", z: "World" }], +}; + +``` + +### Eval output +(kind: ok)
{"x":"Hello","z":"World"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.js new file mode 100644 index 0000000000..d3e83d560c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/nonmutated-spread-props.js @@ -0,0 +1,12 @@ +import {identity, Stringify} from 'shared-runtime'; + +function Component({x, ...rest}) { + const z = rest.z; + identity(z); + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{x: 'Hello', z: 'World'}], +};