-
Notifications
You must be signed in to change notification settings - Fork 50.4k
[compiler] Prune dependencies that are only used by useRef or useState #29653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9e4b4a5
[compiler] Prune dependencies that are only used by useRef or useState
mvitousek e296fda
Update on "[compiler] Prune dependencies that are only used by useRef…
mvitousek 089f405
Update on "[compiler] Prune dependencies that are only used by useRef…
mvitousek 0f504c7
Update on "[compiler] Prune dependencies that are only used by useRef…
mvitousek 23ea0b5
Update on "[compiler] Prune dependencies that are only used by useRef…
mvitousek de2d0cf
Update on "[compiler] Prune dependencies that are only used by useRef…
mvitousek 7fec09c
Update on "[compiler] Prune dependencies that are only used by useRef…
mvitousek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
290 changes: 290 additions & 0 deletions
290
...ackages/babel-plugin-react-compiler/src/ReactiveScopes/PruneInitializationDependencies.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,290 @@ | ||
| /** | ||
| * Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import { CompilerError } from "../CompilerError"; | ||
| import { | ||
| Environment, | ||
| Identifier, | ||
| IdentifierId, | ||
| InstructionId, | ||
| Place, | ||
| ReactiveBlock, | ||
| ReactiveFunction, | ||
| ReactiveInstruction, | ||
| ReactiveScopeBlock, | ||
| ReactiveTerminalStatement, | ||
| getHookKind, | ||
| isUseRefType, | ||
| isUseStateType, | ||
| } from "../HIR"; | ||
| import { eachCallArgument, eachInstructionLValue } from "../HIR/visitors"; | ||
| import DisjointSet from "../Utils/DisjointSet"; | ||
| import { assertExhaustive } from "../Utils/utils"; | ||
| import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; | ||
|
|
||
| /** | ||
| * This pass is built based on the observation by @jbrown215 that arguments | ||
| * to useState and useRef are only used the first time a component is rendered. | ||
| * Any subsequent times, the arguments will be evaluated but ignored. In this pass, | ||
| * we use this fact to improve the output of the compiler by not recomputing values that | ||
| * are only used as arguments (or inputs to arguments to) useState and useRef. | ||
| * | ||
| * This pass isn't yet stress-tested so it's not enabled by default. It's only enabled | ||
| * to support certain debug modes that detect non-idempotent code, since non-idempotent | ||
| * code can "safely" be used if its only passed to useState and useRef. We plan to rewrite | ||
| * this pass in HIR and enable it as an optimization in the future. | ||
| * | ||
| * Algorithm: | ||
| * We take two passes over the reactive function AST. In the first pass, we gather | ||
| * aliases and build relationships between property accesses--the key thing we need | ||
| * to do here is to find that, e.g., $0.x and $1 refer to the same value if | ||
| * $1 = PropertyLoad $0.x. | ||
| * | ||
| * In the second pass, we traverse the AST in reverse order and track how each place | ||
| * is used. If a place is read from in any Terminal, we mark the place as "Update", meaning | ||
| * it is used whenever the component is updated/re-rendered. If a place is read from in | ||
| * a useState or useRef hook call, we mark it as "Create", since it is only used when the | ||
| * component is created. In other instructions, we propagate the inferred place for the | ||
| * instructions lvalues onto any other instructions that are read. | ||
| * | ||
| * Whenever we finish this reverse pass over a reactive block, we can look at the blocks | ||
| * dependencies and see whether the dependencies are used in an "Update" context or only | ||
| * in a "Create" context. If a dependency is create-only, then we can remove that dependency | ||
| * from the block. | ||
| */ | ||
|
|
||
| type CreateUpdate = "Create" | "Update" | "Unknown"; | ||
|
|
||
| type KindMap = Map<IdentifierId, CreateUpdate>; | ||
|
|
||
| class Visitor extends ReactiveFunctionVisitor<CreateUpdate> { | ||
| map: KindMap = new Map(); | ||
| aliases: DisjointSet<IdentifierId>; | ||
| paths: Map<IdentifierId, Map<string, IdentifierId>>; | ||
| env: Environment; | ||
|
|
||
| constructor( | ||
| env: Environment, | ||
| aliases: DisjointSet<IdentifierId>, | ||
| paths: Map<IdentifierId, Map<string, IdentifierId>> | ||
| ) { | ||
| super(); | ||
| this.aliases = aliases; | ||
| this.paths = paths; | ||
| this.env = env; | ||
| } | ||
|
|
||
| join(values: Array<CreateUpdate>): CreateUpdate { | ||
| function join2(l: CreateUpdate, r: CreateUpdate): CreateUpdate { | ||
| if (l === "Update" || r === "Update") { | ||
| return "Update"; | ||
| } else if (l === "Create" || r === "Create") { | ||
| return "Create"; | ||
| } else if (l === "Unknown" || r === "Unknown") { | ||
| return "Unknown"; | ||
| } | ||
| assertExhaustive(r, `Unhandled variable kind ${r}`); | ||
| } | ||
| return values.reduce(join2, "Unknown"); | ||
| } | ||
|
|
||
| isCreateOnlyHook(id: Identifier): boolean { | ||
| return isUseStateType(id) || isUseRefType(id); | ||
| } | ||
|
|
||
| override visitPlace( | ||
| _: InstructionId, | ||
| place: Place, | ||
| state: CreateUpdate | ||
| ): void { | ||
| this.map.set( | ||
| place.identifier.id, | ||
| this.join([state, this.map.get(place.identifier.id) ?? "Unknown"]) | ||
| ); | ||
| } | ||
|
|
||
| override visitBlock(block: ReactiveBlock, state: CreateUpdate): void { | ||
| super.visitBlock([...block].reverse(), state); | ||
| } | ||
|
|
||
| override visitInstruction(instruction: ReactiveInstruction): void { | ||
| const state = this.join( | ||
| [...eachInstructionLValue(instruction)].map( | ||
| (operand) => this.map.get(operand.identifier.id) ?? "Unknown" | ||
| ) | ||
| ); | ||
|
|
||
| const visitCallOrMethodNonArgs = (): void => { | ||
| switch (instruction.value.kind) { | ||
| case "CallExpression": { | ||
| this.visitPlace(instruction.id, instruction.value.callee, state); | ||
| break; | ||
| } | ||
| case "MethodCall": { | ||
| this.visitPlace(instruction.id, instruction.value.property, state); | ||
| this.visitPlace(instruction.id, instruction.value.receiver, state); | ||
| break; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const isHook = (): boolean => { | ||
| let callee = null; | ||
| switch (instruction.value.kind) { | ||
| case "CallExpression": { | ||
| callee = instruction.value.callee.identifier; | ||
| break; | ||
| } | ||
| case "MethodCall": { | ||
| callee = instruction.value.property.identifier; | ||
| break; | ||
| } | ||
| } | ||
| return callee != null && getHookKind(this.env, callee) != null; | ||
| }; | ||
|
|
||
| switch (instruction.value.kind) { | ||
| case "CallExpression": | ||
| case "MethodCall": { | ||
| if ( | ||
| instruction.lvalue && | ||
| this.isCreateOnlyHook(instruction.lvalue.identifier) | ||
| ) { | ||
| [...eachCallArgument(instruction.value.args)].forEach((operand) => | ||
| this.visitPlace(instruction.id, operand, "Create") | ||
| ); | ||
| visitCallOrMethodNonArgs(); | ||
| } else { | ||
| this.traverseInstruction(instruction, isHook() ? "Update" : state); | ||
| } | ||
| break; | ||
| } | ||
| default: { | ||
| this.traverseInstruction(instruction, state); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override visitScope(scope: ReactiveScopeBlock): void { | ||
| const state = this.join( | ||
| [ | ||
| ...scope.scope.declarations.keys(), | ||
| ...[...scope.scope.reassignments.values()].map((ident) => ident.id), | ||
| ].map((id) => this.map.get(id) ?? "Unknown") | ||
| ); | ||
| super.visitScope(scope, state); | ||
| [...scope.scope.dependencies].forEach((ident) => { | ||
| let target: undefined | IdentifierId = | ||
| this.aliases.find(ident.identifier.id) ?? ident.identifier.id; | ||
| ident.path.forEach((key) => { | ||
| target &&= this.paths.get(target)?.get(key); | ||
| }); | ||
| if (target && this.map.get(target) === "Create") { | ||
| scope.scope.dependencies.delete(ident); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| override visitTerminal( | ||
| stmt: ReactiveTerminalStatement, | ||
| state: CreateUpdate | ||
| ): void { | ||
| CompilerError.invariant(state !== "Create", { | ||
| reason: "Visiting a terminal statement with state 'Create'", | ||
| loc: stmt.terminal.loc, | ||
| }); | ||
| super.visitTerminal(stmt, state); | ||
| } | ||
|
|
||
| override visitReactiveFunctionValue( | ||
| _id: InstructionId, | ||
| _dependencies: Array<Place>, | ||
| fn: ReactiveFunction, | ||
| state: CreateUpdate | ||
| ): void { | ||
| visitReactiveFunction(fn, this, state); | ||
| } | ||
| } | ||
|
|
||
| export default function pruneInitializationDependencies( | ||
| fn: ReactiveFunction | ||
| ): void { | ||
| const [aliases, paths] = getAliases(fn); | ||
| visitReactiveFunction(fn, new Visitor(fn.env, aliases, paths), "Update"); | ||
| } | ||
|
|
||
| function update( | ||
| map: Map<IdentifierId, Map<string, IdentifierId>>, | ||
| key: IdentifierId, | ||
| path: string, | ||
| value: IdentifierId | ||
| ): void { | ||
| const inner = map.get(key) ?? new Map(); | ||
| inner.set(path, value); | ||
| map.set(key, inner); | ||
| } | ||
|
|
||
| class AliasVisitor extends ReactiveFunctionVisitor { | ||
| scopeIdentifiers: DisjointSet<IdentifierId> = new DisjointSet<IdentifierId>(); | ||
| scopePaths: Map<IdentifierId, Map<string, IdentifierId>> = new Map(); | ||
|
|
||
| override visitInstruction(instr: ReactiveInstruction): void { | ||
| if ( | ||
| instr.value.kind === "StoreLocal" || | ||
| instr.value.kind === "StoreContext" | ||
| ) { | ||
| this.scopeIdentifiers.union([ | ||
| instr.value.lvalue.place.identifier.id, | ||
| instr.value.value.identifier.id, | ||
| ]); | ||
| } else if ( | ||
| instr.value.kind === "LoadLocal" || | ||
| instr.value.kind === "LoadContext" | ||
| ) { | ||
| instr.lvalue && | ||
| this.scopeIdentifiers.union([ | ||
| instr.lvalue.identifier.id, | ||
| instr.value.place.identifier.id, | ||
| ]); | ||
| } else if (instr.value.kind === "PropertyLoad") { | ||
| instr.lvalue && | ||
| update( | ||
| this.scopePaths, | ||
| instr.value.object.identifier.id, | ||
| instr.value.property, | ||
| instr.lvalue.identifier.id | ||
| ); | ||
| } else if (instr.value.kind === "PropertyStore") { | ||
| update( | ||
| this.scopePaths, | ||
| instr.value.object.identifier.id, | ||
| instr.value.property, | ||
| instr.value.value.identifier.id | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function getAliases( | ||
| fn: ReactiveFunction | ||
| ): [DisjointSet<IdentifierId>, Map<IdentifierId, Map<string, IdentifierId>>] { | ||
| const visitor = new AliasVisitor(); | ||
| visitReactiveFunction(fn, visitor, null); | ||
| let disjoint = visitor.scopeIdentifiers; | ||
| let scopePaths = new Map<IdentifierId, Map<string, IdentifierId>>(); | ||
| for (const [key, value] of visitor.scopePaths) { | ||
| for (const [path, id] of value) { | ||
| update( | ||
| scopePaths, | ||
| disjoint.find(key) ?? key, | ||
| path, | ||
| disjoint.find(id) ?? id | ||
| ); | ||
| } | ||
| } | ||
| return [disjoint, scopePaths]; | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmmm it seems like this is tracking only a single possible value for an object.property path at a time, but different branches of execution could have different values. Why do we need to track property loads and stores for this particular pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ultimate purpose of doing this alias/property load analysis is to link the identifiers that we identify (via the backwards traversal in the initialization dependencies Visitor) as being only used as input to useState/useRef, with the dependency expressions that we want to prune. For example, in this example, we can look at the state of the program after PruneAlwaysInvalidatingScopes (which is the pass that runs before this one). By traversing backwards from the useState call, we can find that $14 is only ever used as input to useState, so it doesn't have to be a dependency in the reactive scope corresponding to
f(props.x). But the precise expression that is in the reactive scopes dependencies is $11.x. We need this load/store analysis to determine that $11.x and $14 refer to the same value, so we can prune $11.x from the dependencies.I think the key thing I missed here was that the alias visitor should skip terminals! Since it's just an optimization to strip dependencies, and since most cases where this analysis strips dependencies are inherently unconditional (like
useState(f(props.x))), we should just not track aliasing that might be conditional.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes! Just running up to the first terminal makes a ton of sense.