-
Notifications
You must be signed in to change notification settings - Fork 46.8k
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
[compiler] General-purpose function outlining #30331
Changes from all commits
d852a12
2848418
cdffa69
0f30a14
b78cd6e
28278fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
/** | ||
* 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 { HIRFunction } from "../HIR"; | ||
|
||
export function outlineFunctions(fn: HIRFunction): void { | ||
for (const [, block] of fn.body.blocks) { | ||
for (const instr of block.instructions) { | ||
const { value } = instr; | ||
|
||
if ( | ||
value.kind === "FunctionExpression" || | ||
value.kind === "ObjectMethod" | ||
) { | ||
// Recurse in case there are inner functions which can be outlined | ||
outlineFunctions(value.loweredFunc.func); | ||
} | ||
|
||
if ( | ||
value.kind === "FunctionExpression" && | ||
value.loweredFunc.dependencies.length === 0 && | ||
value.loweredFunc.func.context.length === 0 && | ||
// TODO: handle outlining named functions | ||
value.loweredFunc.func.id === null | ||
) { | ||
const loweredFunc = value.loweredFunc.func; | ||
|
||
const id = fn.env.generateGloballyUniqueIdentifierName(loweredFunc.id); | ||
loweredFunc.id = id.value; | ||
|
||
fn.env.outlineFunction(loweredFunc, null); | ||
instr.value = { | ||
kind: "LoadGlobal", | ||
binding: { | ||
kind: "Global", | ||
name: id.value, | ||
}, | ||
loc: value.loc, | ||
}; | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,12 @@ | |
|
||
import * as t from "@babel/types"; | ||
import { createHmac } from "crypto"; | ||
import { pruneHoistedContexts, pruneUnusedLValues, pruneUnusedLabels } from "."; | ||
import { | ||
pruneHoistedContexts, | ||
pruneUnusedLValues, | ||
pruneUnusedLabels, | ||
renameVariables, | ||
} from "."; | ||
import { CompilerError, ErrorSeverity } from "../CompilerError"; | ||
import { Environment, EnvironmentConfig, ExternalFunction } from "../HIR"; | ||
import { | ||
|
@@ -36,6 +41,7 @@ import { | |
ValidIdentifierName, | ||
getHookKind, | ||
makeIdentifierName, | ||
promoteTemporary, | ||
} from "../HIR/HIR"; | ||
import { printIdentifier, printPlace } from "../HIR/PrintHIR"; | ||
import { eachPatternOperand } from "../HIR/visitors"; | ||
|
@@ -45,6 +51,8 @@ import { assertExhaustive } from "../Utils/utils"; | |
import { buildReactiveFunction } from "./BuildReactiveFunction"; | ||
import { SINGLE_CHILD_FBT_TAGS } from "./MemoizeFbtAndMacroOperandsInSameScope"; | ||
import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; | ||
import { ReactFunctionType } from "../HIR/Environment"; | ||
import { logReactiveFunction } from "../Utils/logger"; | ||
|
||
export const MEMO_CACHE_SENTINEL = "react.memo_cache_sentinel"; | ||
export const EARLY_RETURN_SENTINEL = "react.early_return_sentinel"; | ||
|
@@ -85,6 +93,11 @@ export type CodegenFunction = { | |
* because they were part of a pruned memo block. | ||
*/ | ||
prunedMemoValues: number; | ||
|
||
outlined: Array<{ | ||
fn: CodegenFunction; | ||
type: ReactFunctionType | null; | ||
}>; | ||
}; | ||
|
||
export function codegenFunction( | ||
|
@@ -258,6 +271,40 @@ export function codegenFunction( | |
compiled.body.body.unshift(test); | ||
} | ||
|
||
const outlined: CodegenFunction["outlined"] = []; | ||
for (const { fn: outlinedFunction, type } of cx.env.getOutlinedFunctions()) { | ||
const reactiveFunction = buildReactiveFunction(outlinedFunction); | ||
pruneUnusedLabels(reactiveFunction); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels a little weird to me to be running these passes on outlined functions in the codegen phase. I wonder if it's possible to store the outlined functions in the reactiveFunction language from the get-go and then run the pruning passes on outlined functions at the same time we run it over reactive functions generally? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's a reasonable alternative. I did it this way because we run these cleanup passes for other function expressions inside codegen, so it keeps things more consistent. Eventually the entire pipeline will be HIR-based, and each of these passes can recurse into function expressions and we won't need this anymore. So using HIR for the outlined functions aligns more w the long-term direction, i think. |
||
pruneUnusedLValues(reactiveFunction); | ||
pruneHoistedContexts(reactiveFunction); | ||
|
||
/* | ||
* TODO: temporary function params (due to destructuring) should always be | ||
* promoted so that they can be renamed | ||
*/ | ||
for (const param of reactiveFunction.params) { | ||
const place = param.kind === "Identifier" ? param : param.place; | ||
if (place.identifier.name === null) { | ||
promoteTemporary(place.identifier); | ||
} | ||
} | ||
const identifiers = renameVariables(reactiveFunction); | ||
logReactiveFunction("Outline", reactiveFunction); | ||
const codegen = codegenReactiveFunction( | ||
new Context( | ||
cx.env, | ||
reactiveFunction.id ?? "[[ anonymous ]]", | ||
identifiers | ||
), | ||
reactiveFunction | ||
); | ||
if (codegen.isErr()) { | ||
return codegen; | ||
} | ||
outlined.push({ fn: codegen.unwrap(), type }); | ||
} | ||
compiled.outlined = outlined; | ||
|
||
return compileResult; | ||
} | ||
|
||
|
@@ -306,6 +353,7 @@ function codegenReactiveFunction( | |
memoValues: countMemoBlockVisitor.memoValues, | ||
prunedMemoBlocks: countMemoBlockVisitor.prunedMemoBlocks, | ||
prunedMemoValues: countMemoBlockVisitor.prunedMemoValues, | ||
outlined: [], | ||
}); | ||
} | ||
|
||
|
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.
If we choose to outline the parent function, is there any benefit in outlining its nested functions?
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.
Probably minimal, but I have to imagine it can still be slightly faster to avoid creating a closure unnecessarily.