Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reduce element decorator temp variables #16280

Merged
merged 6 commits into from Feb 14, 2024
Merged

Reduce element decorator temp variables #16280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ddefc1e
optimize: allocate temp variable for every decorated element
JLHwung Feb 14, 2024
ad7add1
update test fixtures
JLHwung Feb 14, 2024
a91381e
refactor: sort decorator info once
JLHwung Feb 14, 2024
740a10c
refactor: simplify maybeExtractDecorators interface
JLHwung Feb 14, 2024
0977e10
refactor: rename maybeExtractDecorators to handleDecoratorExpressions
JLHwung Feb 14, 2024
b86cb8c
unify haveThis and hasThis
JLHwung Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
198 changes: 107 additions & 91 deletions packages/babel-helper-create-class-features-plugin/src/decorators.ts
View file
Open in desktop
Expand Up @@ -455,7 +455,7 @@ const SETTER = 4;

const STATIC_OLD_VERSION = 5; // Before 2023-05
const STATIC = 8; // 1 << 3
const DECORATORS_HAVE_THIS = 16; // 1 << 3
const DECORATORS_HAVE_THIS = 16; // 1 << 4

function getElementKind(element: NodePath<ClassDecoratableElement>): number {
switch (element.node.type) {
Expand All @@ -478,9 +478,9 @@ function getElementKind(element: NodePath<ClassDecoratableElement>): number {

// Information about the decorators applied to an element
interface DecoratorInfo {
// The expressions of the decorators themselves
decorators: t.Expression[];
decoratorsThis: t.Expression[];
// An array of applied decorators or a memoised identifier
decoratorsArray: t.Identifier | t.ArrayExpression | t.Expression;
decoratorsHaveThis: boolean;

// The kind of the decorated value, matches the kind value passed to applyDecs
kind: number;
Expand All @@ -499,50 +499,56 @@ interface DecoratorInfo {
locals: t.Identifier | t.Identifier[] | undefined;
}

// Information about a computed property key. These must be evaluated
// interspersed with decorator expressions, which is why they get added to the
// array of DecoratorInfos later on.
interface ComputedPropInfo {
localComputedNameId: t.Identifier;
keyNode: t.Expression;
}

function isDecoratorInfo(
info: DecoratorInfo | ComputedPropInfo,
): info is DecoratorInfo {
return "decorators" in info;
}

function filteredOrderedDecoratorInfo(
info: (DecoratorInfo | ComputedPropInfo)[],
): DecoratorInfo[] {
const filtered = info.filter(isDecoratorInfo);

/**
* Sort decoration info in the application order:
* - static non-fields
* - instance non-fields
* - static fields
* - instance fields
*
* @param {DecoratorInfo[]} info
* @returns {DecoratorInfo[]} Sorted decoration info
*/
function toSortedDecoratorInfo(info: DecoratorInfo[]): DecoratorInfo[] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR is not depending on tc39/proposal-decorators#524, since any normative change of decorator application ordering should be implemented within toSortedDecoratorInfo.

return [
...filtered.filter(
...info.filter(
el => el.isStatic && el.kind >= ACCESSOR && el.kind <= SETTER,
),
...filtered.filter(
...info.filter(
el => !el.isStatic && el.kind >= ACCESSOR && el.kind <= SETTER,
),
...filtered.filter(el => el.isStatic && el.kind === FIELD),
...filtered.filter(el => !el.isStatic && el.kind === FIELD),
...info.filter(el => el.isStatic && el.kind === FIELD),
...info.filter(el => !el.isStatic && el.kind === FIELD),
];
}

type GenerateDecorationListResult = {
// The zipped decorators array that will be passed to generateDecorationExprs
decs: t.Expression[];
// Whether there are non-empty decorator this values
haveThis: boolean;
};
/**
* Zip decorators and decorator this values into an array
*
* @param {t.Expression[]} decorators
* @param {((t.Expression | undefined)[])} decoratorsThis decorator this values
* @param {DecoratorVersionKind} version
* @returns {GenerateDecorationListResult}
*/
function generateDecorationList(
decorators: t.Expression[],
decoratorsThis: (t.Expression | null)[],
decoratorsThis: (t.Expression | undefined)[],
version: DecoratorVersionKind,
) {
): GenerateDecorationListResult {
const decsCount = decorators.length;
const hasOneThis = decoratorsThis.some(Boolean);
const haveOneThis = decoratorsThis.some(Boolean);
const decs: t.Expression[] = [];
for (let i = 0; i < decsCount; i++) {
if (
(version === "2023-11" ||
(!process.env.BABEL_8_BREAKING && version === "2023-05")) &&
hasOneThis
haveOneThis
) {
decs.push(
decoratorsThis[i] || t.unaryExpression("void", t.numericLiteral(0)),
Expand All @@ -551,21 +557,15 @@ function generateDecorationList(
decs.push(decorators[i]);
}

return { hasThis: hasOneThis, decs };
return { haveThis: haveOneThis, decs };
}

function generateDecorationExprs(
info: (DecoratorInfo | ComputedPropInfo)[],
decorationInfo: DecoratorInfo[],
version: DecoratorVersionKind,
): t.ArrayExpression {
return t.arrayExpression(
filteredOrderedDecoratorInfo(info).map(el => {
const { decs, hasThis } = generateDecorationList(
el.decorators,
el.decoratorsThis,
version,
);

decorationInfo.map(el => {
let flag = el.kind;
if (el.isStatic) {
flag +=
Expand All @@ -574,10 +574,10 @@ function generateDecorationExprs(
? STATIC
: STATIC_OLD_VERSION;
}
if (hasThis) flag += DECORATORS_HAVE_THIS;
if (el.decoratorsHaveThis) flag += DECORATORS_HAVE_THIS;

return t.arrayExpression([
decs.length === 1 ? decs[0] : t.arrayExpression(decs),
el.decoratorsArray,
t.numericLiteral(flag),
el.name,
...(el.privateMethods || []),
Expand All @@ -586,12 +586,10 @@ function generateDecorationExprs(
);
}

function extractElementLocalAssignments(
decorationInfo: (DecoratorInfo | ComputedPropInfo)[],
) {
function extractElementLocalAssignments(decorationInfo: DecoratorInfo[]) {
const localIds: t.Identifier[] = [];

for (const el of filteredOrderedDecoratorInfo(decorationInfo)) {
for (const el of decorationInfo) {
const { locals } = el;

if (Array.isArray(locals)) {
Expand Down Expand Up @@ -905,27 +903,32 @@ function transformClass(
return;
}

const elementDecoratorInfo: (DecoratorInfo | ComputedPropInfo)[] = [];
const elementDecoratorInfo: DecoratorInfo[] = [];

let constructorPath: NodePath<t.ClassMethod> | undefined;
const decoratedPrivateMethods = new Set<string>();

let classInitLocal: t.Identifier, classIdLocal: t.Identifier;

const decoratorsThis = new Map<t.Decorator, t.Expression>();
const maybeExtractDecorators = (
decorators: t.Decorator[],
memoiseInPlace: boolean,
) => {
// Memoise the this value `a.b` of decorator member expressions `@a.b.dec`,
type HandleDecoratorExpressionsResult = {
// whether the whole decorator list requires memoisation
needMemoise: boolean;
// the this value of each decorator if applicable
decoratorsThis: (t.Expression | undefined)[];
};
function handleDecoratorExpressions(
expressions: t.Expression[],
): HandleDecoratorExpressionsResult {
let needMemoise = false;
for (const decorator of decorators) {
const { expression } = decorator;
const decoratorsThis: (t.Expression | null)[] = [];
for (const expression of expressions) {
let object;
if (
(version === "2023-11" ||
(!process.env.BABEL_8_BREAKING && version === "2023-05")) &&
t.isMemberExpression(expression)
) {
let object;
if (
t.isSuper(expression.object) ||
t.isThisExpression(expression.object)
Expand All @@ -937,17 +940,12 @@ function transformClass(
}
object = t.cloneNode(expression.object);
}
decoratorsThis.set(decorator, object);
}
if (!scopeParent.isStatic(expression)) {
needMemoise = true;
if (memoiseInPlace) {
decorator.expression = memoiseExpression(expression, "dec");
}
}
decoratorsThis.push(object);
needMemoise ||= !scopeParent.isStatic(expression);
}
return needMemoise && !memoiseInPlace;
};
return { needMemoise, decoratorsThis };
}

let needsDeclaraionForClassBinding = false;
let classDecorationsFlag = 0;
Expand All @@ -960,14 +958,16 @@ function transformClass(

path.node.decorators = null;

const needMemoise = maybeExtractDecorators(classDecorators, false);
const decoratorExpressions = classDecorators.map(el => el.expression);
const { needMemoise, decoratorsThis } =
handleDecoratorExpressions(decoratorExpressions);

const { hasThis, decs } = generateDecorationList(
classDecorators.map(el => el.expression),
classDecorators.map(dec => decoratorsThis.get(dec)),
const { haveThis, decs } = generateDecorationList(
decoratorExpressions,
decoratorsThis,
version,
);
classDecorationsFlag = hasThis ? 1 : 0;
classDecorationsFlag = haveThis ? 1 : 0;
classDecorations = decs;

if (needMemoise) {
Expand Down Expand Up @@ -1012,15 +1012,38 @@ function transformClass(
}

const { node } = element;
const decorators = element.node.decorators;
const decorators = node.decorators;

const hasDecorators = !!decorators?.length;

const isComputed = "computed" in node && node.computed;

let name = "computedKey";

if (node.key.type === "PrivateName") {
name = node.key.id.name;
} else if (!isComputed && node.key.type === "Identifier") {
name = node.key.name;
}
let decoratorsArray: t.Identifier | t.ArrayExpression | t.Expression;
let decoratorsHaveThis;

if (hasDecorators) {
maybeExtractDecorators(decorators, true);
const decoratorExpressions = decorators.map(d => d.expression);
const { needMemoise, decoratorsThis } =
handleDecoratorExpressions(decoratorExpressions);
const { decs, haveThis } = generateDecorationList(
decoratorExpressions,
decoratorsThis,
version,
);
decoratorsHaveThis = haveThis;
decoratorsArray = decs.length === 1 ? decs[0] : t.arrayExpression(decs);
if (needMemoise) {
decoratorsArray = memoiseExpression(decoratorsArray, name + "Decs");
}
}

const isComputed = "computed" in element.node && element.node.computed;
if (isComputed) {
if (!element.get("key").isConstantExpression()) {
node.key = memoiseExpression(
Expand All @@ -1030,20 +1053,11 @@ function transformClass(
}
}

const kind = getElementKind(element);
const { key } = node;
const { key, static: isStatic } = node;

const isPrivate = key.type === "PrivateName";

const isStatic = element.node.static;

let name = "computedKey";

if (isPrivate) {
name = key.id.name;
} else if (!isComputed && key.type === "Identifier") {
name = key.name;
}
const kind = getElementKind(element);

if (isPrivate && !isStatic) {
if (hasDecorators) {
Expand Down Expand Up @@ -1187,8 +1201,8 @@ function transformClass(

elementDecoratorInfo.push({
kind,
decorators: decorators.map(d => d.expression),
decoratorsThis: decorators.map(d => decoratorsThis.get(d)),
decoratorsArray,
decoratorsHaveThis,
name: nameExpr,
isStatic,
privateMethods,
Expand Down Expand Up @@ -1276,13 +1290,17 @@ function transformClass(
staticFieldInitializerExpressions = [];
}

const sortedElementDecoratorInfo =
toSortedDecoratorInfo(elementDecoratorInfo);

const elementDecorations = generateDecorationExprs(
elementDecoratorInfo,
sortedElementDecoratorInfo,
version,
);

const elementLocals: t.Identifier[] =
extractElementLocalAssignments(elementDecoratorInfo);
const elementLocals: t.Identifier[] = extractElementLocalAssignments(
sortedElementDecoratorInfo,
);

if (protoInitLocal) {
elementLocals.push(protoInitLocal);
Expand Down Expand Up @@ -1407,9 +1425,7 @@ function transformClass(
elementLocals,
classLocals,
elementDecorations,
classDecorationsId
? t.cloneNode(classDecorationsId)
: t.arrayExpression(classDecorations),
classDecorationsId ?? t.arrayExpression(classDecorations),
t.numericLiteral(classDecorationsFlag),
needsInstancePrivateBrandCheck ? lastInstancePrivateName : null,
typeof className === "object" ? className : undefined,
Expand Down
6 changes: 3 additions & 3 deletions ...bel-plugin-proposal-decorators/test/fixtures/2021-12-exported/member-decorator/output.mjs
View file
Open in desktop
@@ -1,8 +1,8 @@
var _dec, _init_x;
_dec = dec;
var _xDecs, _init_x;
_xDecs = dec;
export class A {
static {
[_init_x] = babelHelpers.applyDecs(this, [[_dec, 0, "x"]], []);
[_init_x] = babelHelpers.applyDecs(this, [[_xDecs, 0, "x"]], []);
}
x = _init_x(this);
}
6 changes: 3 additions & 3 deletions ...rs/test/fixtures/2021-12-misc--to-es2015/initProto-existing-derived-constructor/output.js
View file
Open in desktop
@@ -1,6 +1,6 @@
var _initProto, _dec, _A;
var _initProto, _methodDecs, _A;
const dec = () => {};
_dec = deco;
_methodDecs = deco;
class A extends B {
constructor() {
let a = 2;
Expand All @@ -10,4 +10,4 @@ class A extends B {
method() {}
}
_A = A;
[_initProto] = babelHelpers.applyDecs(_A, [[_dec, 2, "method"]], []);
[_initProto] = babelHelpers.applyDecs(_A, [[_methodDecs, 2, "method"]], []);