From bd675b104147594767b56df2756426d032a68583 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 16 Apr 2023 15:01:57 +0200 Subject: [PATCH] Improve memory usage for parameter deoptimizations (#4938) * Try to improve memory consumption when deoptimizing call parameters * Avoid seemingly unnecessary optimization * Merge thisArgs into args so that args is a unique identifier for interactions * Add limit for tracked interactions * Remove caches after clearing them * Add more safeguards * Fix access * 3.20.3-0 --- browser/package.json | 2 +- package-lock.json | 4 +- package.json | 2 +- src/ast/NodeInteractions.ts | 30 +++----- src/ast/nodes/CallExpression.ts | 5 +- src/ast/nodes/ConditionalExpression.ts | 6 +- src/ast/nodes/LogicalExpression.ts | 8 ++- src/ast/nodes/MemberExpression.ts | 25 ++++--- src/ast/nodes/NewExpression.ts | 3 +- src/ast/nodes/TaggedTemplateExpression.ts | 10 ++- src/ast/nodes/ThisExpression.ts | 7 +- src/ast/nodes/shared/CallExpressionBase.ts | 34 ++++----- src/ast/nodes/shared/Expression.ts | 7 +- src/ast/nodes/shared/FunctionBase.ts | 11 +-- src/ast/nodes/shared/FunctionNode.ts | 5 +- src/ast/nodes/shared/MethodBase.ts | 9 +-- src/ast/nodes/shared/MethodTypes.ts | 17 ++--- src/ast/nodes/shared/Node.ts | 6 +- src/ast/nodes/shared/ObjectEntity.ts | 24 ++----- src/ast/nodes/shared/knownGlobals.ts | 6 +- src/ast/utils/PathTracker.ts | 6 +- src/ast/values.ts | 4 +- src/ast/variables/LocalVariable.ts | 3 +- src/ast/variables/ParameterVariable.ts | 82 +++++++++++++++------- src/utils/blank.ts | 7 ++ 25 files changed, 169 insertions(+), 154 deletions(-) diff --git a/browser/package.json b/browser/package.json index 5b04480be8d..57705b02cf9 100644 --- a/browser/package.json +++ b/browser/package.json @@ -1,6 +1,6 @@ { "name": "@rollup/browser", - "version": "3.20.2", + "version": "3.20.3-0", "description": "Next-generation ES module bundler browser build", "main": "dist/rollup.browser.js", "module": "dist/es/rollup.browser.js", diff --git a/package-lock.json b/package-lock.json index 948521c7d28..e119dee9e5d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "rollup", - "version": "3.20.2", + "version": "3.20.3-0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "rollup", - "version": "3.20.2", + "version": "3.20.3-0", "license": "MIT", "bin": { "rollup": "dist/bin/rollup" diff --git a/package.json b/package.json index 33b157c5b45..01a6224664f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "rollup", - "version": "3.20.2", + "version": "3.20.3-0", "description": "Next-generation ES module bundler", "main": "dist/rollup.js", "module": "dist/es/rollup.js", diff --git a/src/ast/NodeInteractions.ts b/src/ast/NodeInteractions.ts index 1d51f52a5b4..012da794330 100644 --- a/src/ast/NodeInteractions.ts +++ b/src/ast/NodeInteractions.ts @@ -6,53 +6,45 @@ export const INTERACTION_ACCESSED = 0; export const INTERACTION_ASSIGNED = 1; export const INTERACTION_CALLED = 2; +// The first argument is the "this" context export interface NodeInteractionAccessed { - args: null; - thisArg: ExpressionEntity | null; + args: readonly [ExpressionEntity | null]; type: typeof INTERACTION_ACCESSED; } export const NODE_INTERACTION_UNKNOWN_ACCESS: NodeInteractionAccessed = { - args: null, - thisArg: null, + args: [null], type: INTERACTION_ACCESSED }; +// The first argument is the "this" context, the second argument the assigned expression export interface NodeInteractionAssigned { - args: readonly [ExpressionEntity]; - thisArg: ExpressionEntity | null; + args: readonly [ExpressionEntity | null, ExpressionEntity]; type: typeof INTERACTION_ASSIGNED; } -export const UNKNOWN_ARG = [UNKNOWN_EXPRESSION] as const; - export const NODE_INTERACTION_UNKNOWN_ASSIGNMENT: NodeInteractionAssigned = { - args: UNKNOWN_ARG, - thisArg: null, + args: [null, UNKNOWN_EXPRESSION], type: INTERACTION_ASSIGNED }; +// The first argument is the "this" context, the other arguments are the actual arguments export interface NodeInteractionCalled { - args: readonly (ExpressionEntity | SpreadElement)[]; - thisArg: ExpressionEntity | null; + args: readonly [ExpressionEntity | null, ...(ExpressionEntity | SpreadElement)[]]; type: typeof INTERACTION_CALLED; withNew: boolean; } -export const NO_ARGS = []; - // While this is technically a call without arguments, we can compare against -// this reference in places where precise values or thisArg would make a +// this reference in places where precise values or this argument would make a // difference export const NODE_INTERACTION_UNKNOWN_CALL: NodeInteractionCalled = { - args: NO_ARGS, - thisArg: null, + args: [null], type: INTERACTION_CALLED, withNew: false }; -// For tracking, called and assigned are uniquely determined by their .args -// while accessed is determined by .thisArg +// For tracking, interactions are uniquely determined by their .args export type NodeInteraction = | NodeInteractionAccessed | NodeInteractionAssigned diff --git a/src/ast/nodes/CallExpression.ts b/src/ast/nodes/CallExpression.ts index 423cf6f94af..d50686fc1f8 100644 --- a/src/ast/nodes/CallExpression.ts +++ b/src/ast/nodes/CallExpression.ts @@ -41,11 +41,12 @@ export default class CallExpression } } this.interaction = { - args: this.arguments, - thisArg: + args: [ this.callee instanceof MemberExpression && !this.callee.variable ? this.callee.object : null, + ...this.arguments + ], type: INTERACTION_CALLED, withNew: false }; diff --git a/src/ast/nodes/ConditionalExpression.ts b/src/ast/nodes/ConditionalExpression.ts index b826841fc24..c8969d5f9ae 100644 --- a/src/ast/nodes/ConditionalExpression.ts +++ b/src/ast/nodes/ConditionalExpression.ts @@ -1,5 +1,5 @@ import type MagicString from 'magic-string'; -import { BLANK } from '../../utils/blank'; +import { BLANK, EMPTY_ARRAY } from '../../utils/blank'; import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers'; import { findFirstOccurrenceOutsideComment, @@ -44,7 +44,9 @@ export default class ConditionalExpression extends NodeBase implements Deoptimiz const unusedBranch = this.usedBranch === this.consequent ? this.alternate : this.consequent; this.usedBranch = null; unusedBranch.deoptimizePath(UNKNOWN_PATH); - for (const expression of this.expressionsToBeDeoptimized) { + const { expressionsToBeDeoptimized } = this; + this.expressionsToBeDeoptimized = EMPTY_ARRAY as unknown as DeoptimizableEntity[]; + for (const expression of expressionsToBeDeoptimized) { expression.deoptimizeCache(); } } diff --git a/src/ast/nodes/LogicalExpression.ts b/src/ast/nodes/LogicalExpression.ts index da90f9943d2..757d8286f1a 100644 --- a/src/ast/nodes/LogicalExpression.ts +++ b/src/ast/nodes/LogicalExpression.ts @@ -1,5 +1,5 @@ import type MagicString from 'magic-string'; -import { BLANK } from '../../utils/blank'; +import { BLANK, EMPTY_ARRAY } from '../../utils/blank'; import { findFirstOccurrenceOutsideComment, findNonWhiteSpace, @@ -54,12 +54,14 @@ export default class LogicalExpression extends NodeBase implements Deoptimizable const unusedBranch = this.usedBranch === this.left ? this.right : this.left; this.usedBranch = null; unusedBranch.deoptimizePath(UNKNOWN_PATH); - for (const expression of this.expressionsToBeDeoptimized) { + const { context, expressionsToBeDeoptimized } = this; + this.expressionsToBeDeoptimized = EMPTY_ARRAY as unknown as DeoptimizableEntity[]; + for (const expression of expressionsToBeDeoptimized) { expression.deoptimizeCache(); } // Request another pass because we need to ensure "include" runs again if // it is rendered - this.context.requestTreeshakingPass(); + context.requestTreeshakingPass(); } } diff --git a/src/ast/nodes/MemberExpression.ts b/src/ast/nodes/MemberExpression.ts index 658c3b8e21a..6babd81a97e 100644 --- a/src/ast/nodes/MemberExpression.ts +++ b/src/ast/nodes/MemberExpression.ts @@ -1,7 +1,7 @@ import type MagicString from 'magic-string'; import type { AstContext } from '../../Module'; import type { NormalizedTreeshakingOptions } from '../../rollup/types'; -import { BLANK } from '../../utils/blank'; +import { BLANK, EMPTY_ARRAY } from '../../utils/blank'; import { errorIllegalImportReassignment, errorMissingExport } from '../../utils/error'; import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers'; import type { DeoptimizableEntity } from '../DeoptimizableEntity'; @@ -101,8 +101,8 @@ export default class MemberExpression declare propertyKey: ObjectPathKey | null; declare type: NodeType.tMemberExpression; variable: Variable | null = null; - protected declare assignmentInteraction: NodeInteractionAssigned & { thisArg: ExpressionEntity }; - private declare accessInteraction: NodeInteractionAccessed & { thisArg: ExpressionEntity }; + protected declare assignmentInteraction: NodeInteractionAssigned; + private declare accessInteraction: NodeInteractionAccessed; private assignmentDeoptimized = false; private bound = false; private expressionsToBeDeoptimized: DeoptimizableEntity[] = []; @@ -152,10 +152,10 @@ export default class MemberExpression } deoptimizeCache(): void { - const expressionsToBeDeoptimized = this.expressionsToBeDeoptimized; - this.expressionsToBeDeoptimized = []; + const { expressionsToBeDeoptimized, object } = this; + this.expressionsToBeDeoptimized = EMPTY_ARRAY as unknown as DeoptimizableEntity[]; this.propertyKey = UnknownKey; - this.object.deoptimizePath(UNKNOWN_PATH); + object.deoptimizePath(UNKNOWN_PATH); for (const expression of expressionsToBeDeoptimized) { expression.deoptimizeCache(); } @@ -185,8 +185,8 @@ export default class MemberExpression if (this.isUndefined) { return undefined; } - this.expressionsToBeDeoptimized.push(origin); - if (path.length < MAX_PATH_DEPTH) { + if (this.propertyKey !== UnknownKey && path.length < MAX_PATH_DEPTH) { + this.expressionsToBeDeoptimized.push(origin); return this.object.getLiteralValueAtPath( [this.getPropertyKey(), ...path], recursionTracker, @@ -213,8 +213,8 @@ export default class MemberExpression if (this.isUndefined) { return [UNDEFINED_EXPRESSION, false]; } - this.expressionsToBeDeoptimized.push(origin); - if (path.length < MAX_PATH_DEPTH) { + if (this.propertyKey !== UnknownKey && path.length < MAX_PATH_DEPTH) { + this.expressionsToBeDeoptimized.push(origin); return this.object.getReturnExpressionWhenCalledAtPath( [this.getPropertyKey(), ...path], interaction, @@ -297,7 +297,7 @@ export default class MemberExpression initialise(): void { this.propertyKey = getResolvablePropertyKey(this); - this.accessInteraction = { args: null, thisArg: this.object, type: INTERACTION_ACCESSED }; + this.accessInteraction = { args: [this.object], type: INTERACTION_ACCESSED }; } isSkippedAsOptional(origin: DeoptimizableEntity): boolean { @@ -340,8 +340,7 @@ export default class MemberExpression setAssignedValue(value: ExpressionEntity) { this.assignmentInteraction = { - args: [value], - thisArg: this.object, + args: [this.object, value], type: INTERACTION_ASSIGNED }; } diff --git a/src/ast/nodes/NewExpression.ts b/src/ast/nodes/NewExpression.ts index 628ef9a2c2b..ccf7caa95ae 100644 --- a/src/ast/nodes/NewExpression.ts +++ b/src/ast/nodes/NewExpression.ts @@ -53,8 +53,7 @@ export default class NewExpression extends NodeBase { initialise(): void { this.interaction = { - args: this.arguments, - thisArg: null, + args: [null, ...this.arguments], type: INTERACTION_CALLED, withNew: true }; diff --git a/src/ast/nodes/TaggedTemplateExpression.ts b/src/ast/nodes/TaggedTemplateExpression.ts index f3e420e0eea..da487e03d91 100644 --- a/src/ast/nodes/TaggedTemplateExpression.ts +++ b/src/ast/nodes/TaggedTemplateExpression.ts @@ -18,6 +18,7 @@ export default class TaggedTemplateExpression extends CallExpressionBase { declare quasi: TemplateLiteral; declare tag: ExpressionNode; declare type: NodeType.tTaggedTemplateExpression; + private declare args: ExpressionEntity[]; bind(): void { super.bind(); @@ -54,7 +55,7 @@ export default class TaggedTemplateExpression extends CallExpressionBase { this.tag.include(context, includeChildrenRecursively); this.quasi.include(context, includeChildrenRecursively); } - this.tag.includeCallArguments(context, this.interaction.args); + this.tag.includeCallArguments(context, this.args); const [returnExpression] = this.getReturnExpression(); if (!returnExpression.included) { returnExpression.include(context, false); @@ -62,9 +63,12 @@ export default class TaggedTemplateExpression extends CallExpressionBase { } initialise(): void { + this.args = [UNKNOWN_EXPRESSION, ...this.quasi.expressions]; this.interaction = { - args: [UNKNOWN_EXPRESSION, ...this.quasi.expressions], - thisArg: this.tag instanceof MemberExpression && !this.tag.variable ? this.tag.object : null, + args: [ + this.tag instanceof MemberExpression && !this.tag.variable ? this.tag.object : null, + ...this.args + ], type: INTERACTION_CALLED, withNew: false }; diff --git a/src/ast/nodes/ThisExpression.ts b/src/ast/nodes/ThisExpression.ts index 2239dd7eea2..f04785e169d 100644 --- a/src/ast/nodes/ThisExpression.ts +++ b/src/ast/nodes/ThisExpression.ts @@ -23,12 +23,7 @@ export default class ThisExpression extends NodeBase { path: ObjectPath, recursionTracker: PathTracker ): void { - // We rewrite the parameter so that a ThisVariable can detect self-mutations - this.variable.deoptimizeArgumentsOnInteractionAtPath( - interaction.thisArg === this ? { ...interaction, thisArg: this.variable } : interaction, - path, - recursionTracker - ); + this.variable.deoptimizeArgumentsOnInteractionAtPath(interaction, path, recursionTracker); } deoptimizePath(path: ObjectPath): void { diff --git a/src/ast/nodes/shared/CallExpressionBase.ts b/src/ast/nodes/shared/CallExpressionBase.ts index 04a498e8c79..be04e290029 100644 --- a/src/ast/nodes/shared/CallExpressionBase.ts +++ b/src/ast/nodes/shared/CallExpressionBase.ts @@ -1,3 +1,4 @@ +import { EMPTY_ARRAY, EMPTY_SET } from '../../../utils/blank'; import type { DeoptimizableEntity } from '../../DeoptimizableEntity'; import type { HasEffectsContext } from '../../ExecutionContext'; import type { NodeInteraction, NodeInteractionCalled } from '../../NodeInteractions'; @@ -15,36 +16,32 @@ import { NodeBase } from './Node'; export default abstract class CallExpressionBase extends NodeBase implements DeoptimizableEntity { protected declare interaction: NodeInteractionCalled; protected returnExpression: [expression: ExpressionEntity, isPure: boolean] | null = null; - private readonly deoptimizableDependentExpressions: DeoptimizableEntity[] = []; - private readonly expressionsToBeDeoptimized = new Set(); + private deoptimizableDependentExpressions: DeoptimizableEntity[] = []; + private expressionsToBeDeoptimized = new Set(); deoptimizeArgumentsOnInteractionAtPath( interaction: NodeInteraction, path: ObjectPath, recursionTracker: PathTracker ): void { - const { args, thisArg } = interaction; + const { args } = interaction; const [returnExpression, isPure] = this.getReturnExpression(recursionTracker); if (isPure) return; + const deoptimizedExpressions = args.filter( + expression => !!expression && expression !== UNKNOWN_EXPRESSION + ) as ExpressionEntity[]; + if (deoptimizedExpressions.length === 0) return; if (returnExpression === UNKNOWN_EXPRESSION) { - thisArg?.deoptimizePath(UNKNOWN_PATH); - if (args) { - for (const argument of args) { - argument.deoptimizePath(UNKNOWN_PATH); - } + for (const expression of deoptimizedExpressions) { + expression.deoptimizePath(UNKNOWN_PATH); } } else { recursionTracker.withTrackedEntityAtPath( path, returnExpression, () => { - if (thisArg) { - this.expressionsToBeDeoptimized.add(thisArg); - } - if (args) { - for (const argument of args) { - this.expressionsToBeDeoptimized.add(argument); - } + for (const expression of deoptimizedExpressions) { + this.expressionsToBeDeoptimized.add(expression); } returnExpression.deoptimizeArgumentsOnInteractionAtPath( interaction, @@ -60,10 +57,13 @@ export default abstract class CallExpressionBase extends NodeBase implements Deo deoptimizeCache(): void { if (this.returnExpression?.[0] !== UNKNOWN_EXPRESSION) { this.returnExpression = UNKNOWN_RETURN_EXPRESSION; - for (const expression of this.deoptimizableDependentExpressions) { + const { deoptimizableDependentExpressions, expressionsToBeDeoptimized } = this; + this.expressionsToBeDeoptimized = EMPTY_SET; + this.deoptimizableDependentExpressions = EMPTY_ARRAY as unknown as DeoptimizableEntity[]; + for (const expression of deoptimizableDependentExpressions) { expression.deoptimizeCache(); } - for (const expression of this.expressionsToBeDeoptimized) { + for (const expression of expressionsToBeDeoptimized) { expression.deoptimizePath(UNKNOWN_PATH); } } diff --git a/src/ast/nodes/shared/Expression.ts b/src/ast/nodes/shared/Expression.ts index 4ef66f3f89a..6728ca26e02 100644 --- a/src/ast/nodes/shared/Expression.ts +++ b/src/ast/nodes/shared/Expression.ts @@ -99,10 +99,7 @@ export const UNKNOWN_RETURN_EXPRESSION: [expression: ExpressionEntity, isPure: b ]; export const deoptimizeInteraction = (interaction: NodeInteraction) => { - interaction.thisArg?.deoptimizePath(UNKNOWN_PATH); - if (interaction.args) { - for (const argument of interaction.args) { - argument.deoptimizePath(UNKNOWN_PATH); - } + for (const argument of interaction.args) { + argument?.deoptimizePath(UNKNOWN_PATH); } }; diff --git a/src/ast/nodes/shared/FunctionBase.ts b/src/ast/nodes/shared/FunctionBase.ts index 216e0918cca..2f735b7970b 100644 --- a/src/ast/nodes/shared/FunctionBase.ts +++ b/src/ast/nodes/shared/FunctionBase.ts @@ -49,16 +49,17 @@ export default abstract class FunctionBase extends NodeBase { const { parameters } = this.scope; const { args } = interaction; let hasRest = false; - for (let position = 0; position < args.length; position++) { + for (let position = 0; position < args.length - 1; position++) { const parameter = this.params[position]; + // Only the "this" argument arg[0] can be null + const argument = args[position + 1]!; if (hasRest || parameter instanceof RestElement) { hasRest = true; - args[position].deoptimizePath(UNKNOWN_PATH); + argument.deoptimizePath(UNKNOWN_PATH); } else if (parameter instanceof Identifier) { - // args[position].deoptimizePath(UNKNOWN_PATH); - parameters[position][0].addEntityToBeDeoptimized(args[position]); + parameters[position][0].addEntityToBeDeoptimized(argument); } else if (parameter) { - args[position].deoptimizePath(UNKNOWN_PATH); + argument.deoptimizePath(UNKNOWN_PATH); } } } else { diff --git a/src/ast/nodes/shared/FunctionNode.ts b/src/ast/nodes/shared/FunctionNode.ts index 3dd98871e36..809b0a64ccb 100644 --- a/src/ast/nodes/shared/FunctionNode.ts +++ b/src/ast/nodes/shared/FunctionNode.ts @@ -36,8 +36,9 @@ export default class FunctionNode extends FunctionBase { recursionTracker: PathTracker ): void { super.deoptimizeArgumentsOnInteractionAtPath(interaction, path, recursionTracker); - if (interaction.type === INTERACTION_CALLED && path.length === 0 && interaction.thisArg) { - this.scope.thisVariable.addEntityToBeDeoptimized(interaction.thisArg); + if (interaction.type === INTERACTION_CALLED && path.length === 0 && interaction.args[0]) { + // args[0] is the "this" argument + this.scope.thisVariable.addEntityToBeDeoptimized(interaction.args[0]); } } diff --git a/src/ast/nodes/shared/MethodBase.ts b/src/ast/nodes/shared/MethodBase.ts index 7cfea081226..1cb887c9d1c 100644 --- a/src/ast/nodes/shared/MethodBase.ts +++ b/src/ast/nodes/shared/MethodBase.ts @@ -5,7 +5,6 @@ import { INTERACTION_ACCESSED, INTERACTION_ASSIGNED, INTERACTION_CALLED, - NO_ARGS, NODE_INTERACTION_UNKNOWN_CALL } from '../../NodeInteractions'; import { @@ -39,8 +38,7 @@ export default class MethodBase extends NodeBase implements DeoptimizableEntity if (interaction.type === INTERACTION_ACCESSED && this.kind === 'get' && path.length === 0) { return this.value.deoptimizeArgumentsOnInteractionAtPath( { - args: NO_ARGS, - thisArg: interaction.thisArg, + args: interaction.args, type: INTERACTION_CALLED, withNew: false }, @@ -52,7 +50,6 @@ export default class MethodBase extends NodeBase implements DeoptimizableEntity return this.value.deoptimizeArgumentsOnInteractionAtPath( { args: interaction.args, - thisArg: interaction.thisArg, type: INTERACTION_CALLED, withNew: false }, @@ -110,8 +107,7 @@ export default class MethodBase extends NodeBase implements DeoptimizableEntity return this.value.hasEffectsOnInteractionAtPath( EMPTY_PATH, { - args: NO_ARGS, - thisArg: interaction.thisArg, + args: interaction.args, type: INTERACTION_CALLED, withNew: false }, @@ -124,7 +120,6 @@ export default class MethodBase extends NodeBase implements DeoptimizableEntity EMPTY_PATH, { args: interaction.args, - thisArg: interaction.thisArg, type: INTERACTION_CALLED, withNew: false }, diff --git a/src/ast/nodes/shared/MethodTypes.ts b/src/ast/nodes/shared/MethodTypes.ts index 7830908f7a7..b1b01ec0890 100644 --- a/src/ast/nodes/shared/MethodTypes.ts +++ b/src/ast/nodes/shared/MethodTypes.ts @@ -33,18 +33,15 @@ export class Method extends ExpressionEntity { super(); } - deoptimizeArgumentsOnInteractionAtPath( - { type, thisArg }: NodeInteraction, - path: ObjectPath - ): void { + deoptimizeArgumentsOnInteractionAtPath({ args, type }: NodeInteraction, path: ObjectPath): void { if (type === INTERACTION_CALLED && path.length === 0 && this.description.mutatesSelfAsArray) { - thisArg?.deoptimizePath(UNKNOWN_INTEGER_PATH); + args[0]?.deoptimizePath(UNKNOWN_INTEGER_PATH); } } getReturnExpressionWhenCalledAtPath( path: ObjectPath, - { thisArg }: NodeInteractionCalled + { args }: NodeInteractionCalled ): [expression: ExpressionEntity, isPure: boolean] { if (path.length > 0) { return UNKNOWN_RETURN_EXPRESSION; @@ -52,7 +49,7 @@ export class Method extends ExpressionEntity { return [ this.description.returnsPrimitive || (this.description.returns === 'self' - ? thisArg || UNKNOWN_EXPRESSION + ? args[0] || UNKNOWN_EXPRESSION : this.description.returns()), false ]; @@ -68,10 +65,10 @@ export class Method extends ExpressionEntity { return true; } if (type === INTERACTION_CALLED) { - const { args, thisArg } = interaction; + const { args } = interaction; if ( this.description.mutatesSelfAsArray === true && - thisArg?.hasEffectsOnInteractionAtPath( + args[0]?.hasEffectsOnInteractionAtPath( UNKNOWN_INTEGER_PATH, NODE_INTERACTION_UNKNOWN_ASSIGNMENT, context @@ -82,7 +79,7 @@ export class Method extends ExpressionEntity { if (this.description.callsArgs) { for (const argumentIndex of this.description.callsArgs) { if ( - args[argumentIndex]?.hasEffectsOnInteractionAtPath( + args[argumentIndex + 1]?.hasEffectsOnInteractionAtPath( EMPTY_PATH, NODE_INTERACTION_UNKNOWN_CALL, context diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts index c450d4bb35a..6c7d26eefda 100644 --- a/src/ast/nodes/shared/Node.ts +++ b/src/ast/nodes/shared/Node.ts @@ -65,7 +65,7 @@ export interface Node extends Entity { /** * Special version of hasEffects for assignment left-hand sides which ensures * that accessor effects are checked as well. This is necessary to do from the - * child so that member expressions can use the correct thisArg value. + * child so that member expressions can use the correct this value. * setAssignedValue needs to be called during initialise to use this. */ hasEffectsAsAssignmentTarget(context: HasEffectsContext, checkAccess: boolean): boolean; @@ -84,7 +84,7 @@ export interface Node extends Entity { /** * Special version of include for assignment left-hand sides which ensures * that accessors are handled correctly. This is necessary to do from the - * child so that member expressions can use the correct thisArg value. + * child so that member expressions can use the correct this value. * setAssignedValue needs to be called during initialise to use this. */ includeAsAssignmentTarget( @@ -309,7 +309,7 @@ export class NodeBase extends ExpressionEntity implements ExpressionNode { } setAssignedValue(value: ExpressionEntity): void { - this.assignmentInteraction = { args: [value], thisArg: null, type: INTERACTION_ASSIGNED }; + this.assignmentInteraction = { args: [null, value], type: INTERACTION_ASSIGNED }; } shouldBeIncluded(context: InclusionContext): boolean { diff --git a/src/ast/nodes/shared/ObjectEntity.ts b/src/ast/nodes/shared/ObjectEntity.ts index ae411ca39ae..ac981bc3ef0 100644 --- a/src/ast/nodes/shared/ObjectEntity.ts +++ b/src/ast/nodes/shared/ObjectEntity.ts @@ -12,6 +12,7 @@ import { } from '../../utils/PathTracker'; import type { LiteralValueOrUnknown } from './Expression'; import { + deoptimizeInteraction, ExpressionEntity, UNKNOWN_EXPRESSION, UNKNOWN_RETURN_EXPRESSION, @@ -95,7 +96,7 @@ export class ObjectEntity extends ExpressionEntity { recursionTracker: PathTracker ): void { const [key, ...subPath] = path; - const { args, thisArg, type } = interaction; + const { args, type } = interaction; if ( this.hasLostTrack || @@ -104,12 +105,7 @@ export class ObjectEntity extends ExpressionEntity { (this.hasUnknownDeoptimizedProperty || (typeof key === 'string' && this.deoptimizedPaths[key]))) ) { - thisArg?.deoptimizePath(UNKNOWN_PATH); - if (args) { - for (const argument of args) { - argument.deoptimizePath(UNKNOWN_PATH); - } - } + deoptimizeInteraction(interaction); return; } @@ -133,11 +129,8 @@ export class ObjectEntity extends ExpressionEntity { } } if (!this.immutable) { - if (thisArg) { - this.additionalExpressionsToBeDeoptimized.add(thisArg); - } - if (args) { - for (const argument of args) { + for (const argument of args) { + if (argument) { this.additionalExpressionsToBeDeoptimized.add(argument); } } @@ -166,11 +159,8 @@ export class ObjectEntity extends ExpressionEntity { } } if (!this.immutable) { - if (thisArg) { - this.additionalExpressionsToBeDeoptimized.add(thisArg); - } - if (args) { - for (const argument of args) { + for (const argument of args) { + if (argument) { this.additionalExpressionsToBeDeoptimized.add(argument); } } diff --git a/src/ast/nodes/shared/knownGlobals.ts b/src/ast/nodes/shared/knownGlobals.ts index 5fd1b3233f7..21d4e751af2 100644 --- a/src/ast/nodes/shared/knownGlobals.ts +++ b/src/ast/nodes/shared/knownGlobals.ts @@ -60,14 +60,14 @@ const PF: GlobalDescription = { const MUTATES_ARG_WITHOUT_ACCESSOR: GlobalDescription = { __proto__: null, [ValueProperties]: { - deoptimizeArgumentsOnCall({ args: [firstArgument] }: NodeInteractionCalled) { + deoptimizeArgumentsOnCall({ args: [, firstArgument] }: NodeInteractionCalled) { firstArgument?.deoptimizePath(UNKNOWN_PATH); }, getLiteralValue: getTruthyLiteralValue, hasEffectsWhenCalled({ args }, context) { return ( - args.length === 0 || - args[0].hasEffectsOnInteractionAtPath( + args.length <= 1 || + args[1].hasEffectsOnInteractionAtPath( UNKNOWN_NON_ACCESSOR_PATH, NODE_INTERACTION_UNKNOWN_ASSIGNMENT, context diff --git a/src/ast/utils/PathTracker.ts b/src/ast/utils/PathTracker.ts index f80ffa55051..406ffd661c6 100644 --- a/src/ast/utils/PathTracker.ts +++ b/src/ast/utils/PathTracker.ts @@ -90,18 +90,18 @@ export class DiscriminatedPathTracker { trackEntityAtPathAndGetIfTracked( path: ObjectPath, discriminator: unknown, - entity: Entity + entity: unknown ): boolean { let currentPaths = this.entityPaths; for (const pathSegment of path) { currentPaths = currentPaths[pathSegment] = currentPaths[pathSegment] || - Object.create(null, { [EntitiesKey]: { value: new Map>() } }); + Object.create(null, { [EntitiesKey]: { value: new Map>() } }); } const trackedEntities = getOrCreate( currentPaths[EntitiesKey], discriminator, - getNewSet + getNewSet ); if (trackedEntities.has(entity)) return true; trackedEntities.add(entity); diff --git a/src/ast/values.ts b/src/ast/values.ts index 3a479a29136..0dc21c74dde 100644 --- a/src/ast/values.ts +++ b/src/ast/values.ts @@ -156,9 +156,9 @@ const returnsString: RawMemberDescription = { const stringReplace: RawMemberDescription = { value: { hasEffectsWhenCalled({ args }, context) { - const argument1 = args[1]; + const argument1 = args[2]; return ( - args.length < 2 || + args.length < 3 || (typeof argument1.getLiteralValueAtPath(EMPTY_PATH, SHARED_RECURSION_TRACKER, { deoptimizeCache() {} }) === 'symbol' && diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts index 6225f6bb376..2426d4cd2a9 100644 --- a/src/ast/variables/LocalVariable.ts +++ b/src/ast/variables/LocalVariable.ts @@ -1,4 +1,5 @@ import type { AstContext, default as Module } from '../../Module'; +import { EMPTY_ARRAY } from '../../utils/blank'; import type { DeoptimizableEntity } from '../DeoptimizableEntity'; import type { HasEffectsContext, InclusionContext } from '../ExecutionContext'; import { createInclusionContext } from '../ExecutionContext'; @@ -89,7 +90,7 @@ export default class LocalVariable extends Variable { if (!this.isReassigned) { this.isReassigned = true; const expressionsToBeDeoptimized = this.expressionsToBeDeoptimized; - this.expressionsToBeDeoptimized = []; + this.expressionsToBeDeoptimized = EMPTY_ARRAY as unknown as DeoptimizableEntity[]; for (const expression of expressionsToBeDeoptimized) { expression.deoptimizeCache(); } diff --git a/src/ast/variables/ParameterVariable.ts b/src/ast/variables/ParameterVariable.ts index 458d4836be0..73d7ada021f 100644 --- a/src/ast/variables/ParameterVariable.ts +++ b/src/ast/variables/ParameterVariable.ts @@ -1,5 +1,7 @@ import type { AstContext } from '../../Module'; +import { EMPTY_ARRAY } from '../../utils/blank'; import type { NodeInteraction } from '../NodeInteractions'; +import { INTERACTION_CALLED } from '../NodeInteractions'; import type ExportDefaultDeclaration from '../nodes/ExportDefaultDeclaration'; import type Identifier from '../nodes/Identifier'; import type { ExpressionEntity } from '../nodes/shared/Expression'; @@ -10,7 +12,7 @@ import { } from '../nodes/shared/Expression'; import type { ObjectPath, ObjectPathKey } from '../utils/PathTracker'; import { - DiscriminatedPathTracker, + PathTracker, SHARED_RECURSION_TRACKER, UNKNOWN_PATH, UnknownKey @@ -22,11 +24,17 @@ interface DeoptimizationInteraction { path: ObjectPath; } +const MAX_TRACKED_INTERACTIONS = 20; +const NO_INTERACTIONS = EMPTY_ARRAY as unknown as DeoptimizationInteraction[]; +const UNKNOWN_DEOPTIMIZED_FIELD = new Set([UnknownKey]); +const EMPTY_PATH_TRACKER = new PathTracker(); +const UNKNOWN_DEOPTIMIZED_ENTITY = new Set([UNKNOWN_EXPRESSION]); + export default class ParameterVariable extends LocalVariable { - private readonly deoptimizationInteractions: DeoptimizationInteraction[] = []; - private readonly deoptimizations = new DiscriminatedPathTracker(); - private readonly deoptimizedFields = new Set(); - private readonly entitiesToBeDeoptimized = new Set(); + private deoptimizationInteractions: DeoptimizationInteraction[] = []; + private deoptimizations = new PathTracker(); + private deoptimizedFields = new Set(); + private entitiesToBeDeoptimized = new Set(); constructor( name: string, @@ -37,40 +45,55 @@ export default class ParameterVariable extends LocalVariable { } addEntityToBeDeoptimized(entity: ExpressionEntity): void { - if (this.deoptimizedFields.has(UnknownKey)) { + if (entity === UNKNOWN_EXPRESSION) { + // As unknown expressions fully deoptimize all interactions, we can clear + // the interaction cache at this point provided we keep this optimization + // in mind when adding new interactions + if (!this.entitiesToBeDeoptimized.has(UNKNOWN_EXPRESSION)) { + this.entitiesToBeDeoptimized.add(UNKNOWN_EXPRESSION); + for (const { interaction } of this.deoptimizationInteractions) { + deoptimizeInteraction(interaction); + } + this.deoptimizationInteractions = NO_INTERACTIONS; + } + } else if (this.deoptimizedFields.has(UnknownKey)) { + // This means that we already deoptimized all interactions and no longer + // track them entity.deoptimizePath(UNKNOWN_PATH); - } else { + } else if (!this.entitiesToBeDeoptimized.has(entity)) { + this.entitiesToBeDeoptimized.add(entity); for (const field of this.deoptimizedFields) { entity.deoptimizePath([field]); } + for (const { interaction, path } of this.deoptimizationInteractions) { + entity.deoptimizeArgumentsOnInteractionAtPath(interaction, path, SHARED_RECURSION_TRACKER); + } } - for (const { interaction, path } of this.deoptimizationInteractions) { - entity.deoptimizeArgumentsOnInteractionAtPath(interaction, path, SHARED_RECURSION_TRACKER); - } - this.entitiesToBeDeoptimized.add(entity); } deoptimizeArgumentsOnInteractionAtPath(interaction: NodeInteraction, path: ObjectPath): void { // For performance reasons, we fully deoptimize all deeper interactions - if (path.length >= 2) { + if ( + path.length >= 2 || + this.entitiesToBeDeoptimized.has(UNKNOWN_EXPRESSION) || + this.deoptimizationInteractions.length >= MAX_TRACKED_INTERACTIONS || + (path.length === 1 && + (this.deoptimizedFields.has(UnknownKey) || + (interaction.type === INTERACTION_CALLED && this.deoptimizedFields.has(path[0])))) + ) { deoptimizeInteraction(interaction); return; } - if ( - interaction.thisArg && - !this.deoptimizations.trackEntityAtPathAndGetIfTracked( - path, - interaction.args, - interaction.thisArg - ) - ) { + if (!this.deoptimizations.trackEntityAtPathAndGetIfTracked(path, interaction.args)) { for (const entity of this.entitiesToBeDeoptimized) { entity.deoptimizeArgumentsOnInteractionAtPath(interaction, path, SHARED_RECURSION_TRACKER); } - this.deoptimizationInteractions.push({ - interaction, - path - }); + if (!this.entitiesToBeDeoptimized.has(UNKNOWN_EXPRESSION)) { + this.deoptimizationInteractions.push({ + interaction, + path + }); + } } } @@ -84,8 +107,17 @@ export default class ParameterVariable extends LocalVariable { } this.deoptimizedFields.add(key); for (const entity of this.entitiesToBeDeoptimized) { + // We do not need a recursion tracker here as we already track whether + // this field is deoptimized entity.deoptimizePath(path); } + if (key === UnknownKey) { + // save some memory + this.deoptimizationInteractions = NO_INTERACTIONS; + this.deoptimizations = EMPTY_PATH_TRACKER; + this.deoptimizedFields = UNKNOWN_DEOPTIMIZED_FIELD; + this.entitiesToBeDeoptimized = UNKNOWN_DEOPTIMIZED_ENTITY; + } } getReturnExpressionWhenCalledAtPath( @@ -97,7 +129,7 @@ export default class ParameterVariable extends LocalVariable { if (path.length === 0) { this.deoptimizePath(UNKNOWN_PATH); } else if (!this.deoptimizedFields.has(path[0])) { - this.deoptimizePath(path.slice(0, 1)); + this.deoptimizePath([path[0]]); } return UNKNOWN_RETURN_EXPRESSION; } diff --git a/src/utils/blank.ts b/src/utils/blank.ts index a0741f4a212..01c6bd60c74 100644 --- a/src/utils/blank.ts +++ b/src/utils/blank.ts @@ -1,3 +1,10 @@ export const BLANK: Record = Object.freeze(Object.create(null)); export const EMPTY_OBJECT = Object.freeze({}); export const EMPTY_ARRAY = Object.freeze([]); +export const EMPTY_SET = Object.freeze( + new (class extends Set { + add(): never { + throw new Error('Cannot add to empty set'); + } + })() +);