Skip to content

Commit

Permalink
Reapply object tree-shaking (#5737)
Browse files Browse the repository at this point in the history
* Reapply object tree-shaking

This reverts commit 10bc150.

* Only rely on stable property paths when deoptimizing member expressions

Literal values may be deoptimized later. At that point, we would otherwise need
to deoptimize again. For hasEffects, we can use dynamic literal values as those
checks are repeatedly executed.

Resolves #5735

* Do not mark unused rest parameters as undefined

resolves #5739

* add an test for issue 5734

* update linting

* reduce the count of includePath for ClassBody

* Split includePath into include, includeNode, includePath

- include is for re-including on every pass
- includePath is only for including specific non-empty paths once
- includeNode is for triggering logic on first include, like includePath.

* update test

* Test if using a shared prototype function can improve performance

* Use trivial includeNode method where possible

* Remove unnecessary applyDeoptimizations calls

* Move applyDeoptimizations check from include to includeNode

This should avoid an unnecessary check.

* Only deoptimize body for ArrowFunctionExpression

For block scopes, this is not needed.

* Handle accessed outside variables on includeNode rather than applyDeoptimizations

* If a node has not special logic on include or deoptimize, replace check

* See if we can reduce the number of tree-shaking passes

The idea is to again eagerly include variable inits, but not in a nested call
but in a loop at the end of the regular pass.

* Ensure that deoptimizing a MemberExpression does not crash

* Ensure the "in" operators includes is second operand

* When including all children of a CallExpression, also include all paths

* 4.30.0-0

* Make sure to use builtins when building for Node

Solves an issue when linking with pnpm.

* Revert making objects truthy for now

This solves an issue with TypeScript enums. A proper solution should be to
improve tracking of uninitialized vars.

* 4.30.0-1

* Include variable inits per module instead of per pass

This should reduce peak memory consumption.

* Skip checking for effects in functions with side effects

* Do not cache known paramter value as frozen

Adding the fallback on each call is not more expensive.

* Limit number of tracked deoptimized fields for parameters

* 4.31.0-0

* Remove unnecessary array conversion

* Only track top-level inclusions for parameters

This is a massive performance improvement especially in scenarios with recursive
calls where different properties of a function argument are passed to the same
function.

* Only track arguments in the ArgumentsVariable if referenced

This should save some memory which is rarely needed.

* 4.33.0-0

---------

Co-authored-by: XiaoPi <[email protected]>
  • Loading branch information
lukastaegert and TrickyPi authored Feb 1, 2025
1 parent 494483e commit d7062ef
Show file tree
Hide file tree
Showing 372 changed files with 95,491 additions and 1,310 deletions.
2 changes: 1 addition & 1 deletion rollup.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const treeshake = {
const nodePlugins: readonly Plugin[] = [
replace(fsEventsReplacement),
alias(moduleAliases),
nodeResolve(),
nodeResolve({ preferBuiltins: true }),
json(),
string({ include: '**/*.md' }),
commonjs({
Expand Down
6 changes: 3 additions & 3 deletions scripts/ast-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export const AST_NODES = {
'parameters',
`scope.addParameterVariables(
parameters.map(
parameter => parameter.declare('parameter', UNKNOWN_EXPRESSION) as ParameterVariable[]
parameter => parameter.declare('parameter', EMPTY_PATH, UNKNOWN_EXPRESSION) as ParameterVariable[]
),
parameters[parameters.length - 1] instanceof RestElement
);`
Expand Down Expand Up @@ -153,7 +153,7 @@ export const AST_NODES = {
['body', 'Node']
],
postProcessFields: {
param: ['parameter', "parameter?.declare('parameter', UNKNOWN_EXPRESSION)"]
param: ['parameter', "parameter?.declare('parameter', EMPTY_PATH, UNKNOWN_EXPRESSION)"]
},
scopes: {
body: 'scope.bodyScope'
Expand Down Expand Up @@ -310,7 +310,7 @@ export const AST_NODES = {
'parameters',
`scope.addParameterVariables(
parameters.map(
parameter => parameter.declare('parameter', UNKNOWN_EXPRESSION) as ParameterVariable[]
parameter => parameter.declare('parameter', EMPTY_PATH, UNKNOWN_EXPRESSION) as ParameterVariable[]
),
parameters[parameters.length - 1] instanceof RestElement
);`
Expand Down
1 change: 1 addition & 0 deletions scripts/generate-buffer-parsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ import type { Node, NodeBase } from './nodes/shared/Node';
import type ChildScope from './scopes/ChildScope';
import type ModuleScope from './scopes/ModuleScope';
import TrackingScope from './scopes/TrackingScope';
import { EMPTY_PATH } from './utils/PathTracker';
import type ParameterVariable from './variables/ParameterVariable';
export function convertProgram(
Expand Down
6 changes: 2 additions & 4 deletions scripts/prepare-release.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,8 @@ function getDummyLogSection(headline, pr) {
* @return {Promise<void>}
*/
async function installDependenciesAndLint() {
await Promise.all([
runWithEcho('npm', ['ci', '--ignore-scripts']),
runWithEcho('npm', ['run', 'check-audit'])
]);
await runWithEcho('npm', ['ci', '--ignore-scripts']);
await runWithEcho('npm', ['run', 'check-audit']);
await runWithEcho('npm', ['run', 'ci:lint']);
}

Expand Down
12 changes: 10 additions & 2 deletions src/Graph.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import flru from 'flru';
import { createInclusionContext } from './ast/ExecutionContext';
import type { ExpressionEntity } from './ast/nodes/shared/Expression';
import GlobalScope from './ast/scopes/GlobalScope';
import { PathTracker } from './ast/utils/PathTracker';
import { EntityPathTracker } from './ast/utils/PathTracker';
import type ExternalModule from './ExternalModule';
import Module from './Module';
import { ModuleLoader, type UnresolvedModule } from './ModuleLoader';
Expand Down Expand Up @@ -54,12 +56,13 @@ function normalizeEntryModules(
export default class Graph {
readonly astLru = flru<ProgramNode>(5);
readonly cachedModules = new Map<string, ModuleJSON>();
readonly deoptimizationTracker = new PathTracker();
readonly deoptimizationTracker = new EntityPathTracker();
entryModules: Module[] = [];
readonly fileOperationQueue: Queue;
readonly moduleLoader: ModuleLoader;
readonly modulesById = new Map<string, Module | ExternalModule>();
needsTreeshakingPass = false;
readonly newlyIncludedVariableInits = new Set<ExpressionEntity>();
phase: BuildPhase = BuildPhase.LOAD_AND_PARSE;
readonly pluginDriver: PluginDriver;
readonly pureFunctions: PureFunctions;
Expand Down Expand Up @@ -167,6 +170,7 @@ export default class Graph {
}
if (this.options.treeshake) {
let treeshakingPass = 1;
this.newlyIncludedVariableInits.clear();
do {
timeStart(`treeshaking pass ${treeshakingPass}`, 3);
this.needsTreeshakingPass = false;
Expand All @@ -178,6 +182,10 @@ export default class Graph {
} else {
module.include();
}
for (const entity of this.newlyIncludedVariableInits) {
this.newlyIncludedVariableInits.delete(entity);
entity.include(createInclusionContext(), false);
}
}
}
if (treeshakingPass === 1) {
Expand Down
59 changes: 35 additions & 24 deletions src/Module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { locate } from 'locate-character';
import MagicString from 'magic-string';
import { parseAsync } from '../native';
import { convertProgram } from './ast/bufferParsers';
import type { InclusionContext } from './ast/ExecutionContext';
import { createInclusionContext } from './ast/ExecutionContext';
import { nodeConstructors } from './ast/nodes';
import ExportAllDeclaration from './ast/nodes/ExportAllDeclaration';
Expand All @@ -17,10 +18,12 @@ import Literal from './ast/nodes/Literal';
import type MetaProperty from './ast/nodes/MetaProperty';
import * as NodeType from './ast/nodes/NodeType';
import type Program from './ast/nodes/Program';
import type { ExpressionEntity } from './ast/nodes/shared/Expression';
import type { NodeBase } from './ast/nodes/shared/Node';
import VariableDeclaration from './ast/nodes/VariableDeclaration';
import ModuleScope from './ast/scopes/ModuleScope';
import { type PathTracker, UNKNOWN_PATH } from './ast/utils/PathTracker';
import type { ObjectPath } from './ast/utils/PathTracker';
import { type EntityPathTracker, UNKNOWN_PATH } from './ast/utils/PathTracker';
import ExportDefaultVariable from './ast/variables/ExportDefaultVariable';
import ExportShimVariable from './ast/variables/ExportShimVariable';
import ExternalVariable from './ast/variables/ExternalVariable';
Expand Down Expand Up @@ -116,7 +119,7 @@ export interface AstContext {
addImportMeta: (node: MetaProperty) => void;
addImportSource: (importSource: string) => void;
code: string;
deoptimizationTracker: PathTracker;
deoptimizationTracker: EntityPathTracker;
error: (properties: RollupLog, pos: number) => never;
fileName: string;
getExports: () => string[];
Expand All @@ -128,12 +131,17 @@ export interface AstContext {
importDescriptions: Map<string, ImportDescription>;
includeAllExports: () => void;
includeDynamicImport: (node: ImportExpression) => void;
includeVariableInModule: (variable: Variable) => void;
includeVariableInModule: (
variable: Variable,
path: ObjectPath,
context: InclusionContext
) => void;
log: (level: LogLevel, properties: RollupLog, pos: number) => void;
magicString: MagicString;
manualPureFunctions: PureFunctions;
module: Module; // not to be used for tree-shaking
moduleContext: string;
newlyIncludedVariableInits: Set<ExpressionEntity>;
options: NormalizedInputOptions;
requestTreeshakingPass: () => void;
traceExport: (name: string) => Variable | null;
Expand Down Expand Up @@ -708,16 +716,15 @@ export default class Module {
this.graph.needsTreeshakingPass = true;
}

const inclusionContext = createInclusionContext();
for (const exportName of this.exports.keys()) {
if (includeNamespaceMembers || exportName !== this.info.syntheticNamedExports) {
const variable = this.getVariableForExportName(exportName)[0];
if (!variable) {
return error(logMissingEntryExport(exportName, this.id));
}
this.includeVariable(variable, UNKNOWN_PATH, inclusionContext);
variable.deoptimizePath(UNKNOWN_PATH);
if (!variable.included) {
this.includeVariable(variable);
}
}
}

Expand All @@ -726,7 +733,7 @@ export default class Module {
if (variable) {
variable.deoptimizePath(UNKNOWN_PATH);
if (!variable.included) {
this.includeVariable(variable);
this.includeVariable(variable, UNKNOWN_PATH, inclusionContext);
}
if (variable instanceof ExternalVariable) {
variable.module.reexported = true;
Expand All @@ -752,13 +759,12 @@ export default class Module {

let includeNamespaceMembers = false;

const inclusionContext = createInclusionContext();
for (const name of names) {
const variable = this.getVariableForExportName(name)[0];
if (variable) {
variable.deoptimizePath(UNKNOWN_PATH);
if (!variable.included) {
this.includeVariable(variable);
}
this.includeVariable(variable, UNKNOWN_PATH, inclusionContext);
}

if (!this.exports.has(name) && !this.reexportDescriptions.has(name)) {
Expand Down Expand Up @@ -892,6 +898,7 @@ export default class Module {
manualPureFunctions: this.graph.pureFunctions,
module: this,
moduleContext: this.context,
newlyIncludedVariableInits: this.graph.newlyIncludedVariableInits,
options: this.options,
requestTreeshakingPass: () => (this.graph.needsTreeshakingPass = true),
traceExport: (name: string) => this.getVariableForExportName(name)[0],
Expand Down Expand Up @@ -1336,12 +1343,12 @@ export default class Module {
for (const module of [this, ...this.exportAllModules]) {
if (module instanceof ExternalModule) {
const [externalVariable] = module.getVariableForExportName('*');
externalVariable.include();
externalVariable.includePath(UNKNOWN_PATH, createInclusionContext());
this.includedImports.add(externalVariable);
externalNamespaces.add(externalVariable);
} else if (module.info.syntheticNamedExports) {
const syntheticNamespace = module.getSyntheticNamespace();
syntheticNamespace.include();
syntheticNamespace.includePath(UNKNOWN_PATH, createInclusionContext());
this.includedImports.add(syntheticNamespace);
syntheticNamespaces.add(syntheticNamespace);
}
Expand All @@ -1350,14 +1357,14 @@ export default class Module {
}

private includeDynamicImport(node: ImportExpression): void {
const resolution = (
this.dynamicImports.find(dynamicImport => dynamicImport.node === node) as {
resolution: string | Module | ExternalModule | undefined;
}
).resolution;
const resolution = this.dynamicImports.find(
dynamicImport => dynamicImport.node === node
)!.resolution;

if (resolution instanceof Module) {
resolution.includedDynamicImporters.push(this);
if (!resolution.includedDynamicImporters.includes(this)) {
resolution.includedDynamicImporters.push(this);
}

const importedNames = this.options.treeshake
? node.getDeterministicImportedNames()
Expand All @@ -1371,14 +1378,14 @@ export default class Module {
}
}

private includeVariable(variable: Variable): void {
const variableModule = variable.module;
if (variable.included) {
private includeVariable(variable: Variable, path: ObjectPath, context: InclusionContext): void {
const { included, module: variableModule } = variable;
variable.includePath(path, context);
if (included) {
if (variableModule instanceof Module && variableModule !== this) {
getAndExtendSideEffectModules(variable, this);
}
} else {
variable.include();
this.graph.needsTreeshakingPass = true;
if (variableModule instanceof Module) {
if (!variableModule.isExecuted) {
Expand All @@ -1396,8 +1403,12 @@ export default class Module {
}
}

private includeVariableInModule(variable: Variable): void {
this.includeVariable(variable);
private includeVariableInModule(
variable: Variable,
path: ObjectPath,
context: InclusionContext
): void {
this.includeVariable(variable, path, context);
const variableModule = variable.module;
if (variableModule && variableModule !== this) {
this.includedImports.add(variable);
Expand Down
10 changes: 5 additions & 5 deletions src/ast/ExecutionContext.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Entity } from './Entity';
import type { ExpressionEntity } from './nodes/shared/Expression';
import { DiscriminatedPathTracker, PathTracker } from './utils/PathTracker';
import { DiscriminatedPathTracker, EntityPathTracker } from './utils/PathTracker';
import type ThisVariable from './variables/ThisVariable';

interface ExecutionContextIgnore {
Expand All @@ -23,8 +23,8 @@ export interface InclusionContext extends ControlFlowContext {
}

export interface HasEffectsContext extends ControlFlowContext {
accessed: PathTracker;
assigned: PathTracker;
accessed: EntityPathTracker;
assigned: EntityPathTracker;
brokenFlow: boolean;
called: DiscriminatedPathTracker;
ignore: ExecutionContextIgnore;
Expand All @@ -44,8 +44,8 @@ export function createInclusionContext(): InclusionContext {

export function createHasEffectsContext(): HasEffectsContext {
return {
accessed: new PathTracker(),
assigned: new PathTracker(),
accessed: new EntityPathTracker(),
assigned: new EntityPathTracker(),
brokenFlow: false,
called: new DiscriminatedPathTracker(),
hasBreak: false,
Expand Down
12 changes: 8 additions & 4 deletions src/ast/bufferParsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ import type { Node, NodeBase } from './nodes/shared/Node';
import type ChildScope from './scopes/ChildScope';
import type ModuleScope from './scopes/ModuleScope';
import TrackingScope from './scopes/TrackingScope';
import { EMPTY_PATH } from './utils/PathTracker';
import type ParameterVariable from './variables/ParameterVariable';

export function convertProgram(
Expand Down Expand Up @@ -335,7 +336,8 @@ const bufferParsers: ((node: any, position: number, buffer: AstBuffer) => void)[
const parameters = (node.params = convertNodeList(node, scope, buffer[position + 2], buffer));
scope.addParameterVariables(
parameters.map(
parameter => parameter.declare('parameter', UNKNOWN_EXPRESSION) as ParameterVariable[]
parameter =>
parameter.declare('parameter', EMPTY_PATH, UNKNOWN_EXPRESSION) as ParameterVariable[]
),
parameters[parameters.length - 1] instanceof RestElement
);
Expand Down Expand Up @@ -384,7 +386,7 @@ const bufferParsers: ((node: any, position: number, buffer: AstBuffer) => void)[
const parameterPosition = buffer[position];
const parameter = (node.param =
parameterPosition === 0 ? null : convertNode(node, scope, parameterPosition, buffer));
parameter?.declare('parameter', UNKNOWN_EXPRESSION);
parameter?.declare('parameter', EMPTY_PATH, UNKNOWN_EXPRESSION);
node.body = convertNode(node, scope.bodyScope, buffer[position + 1], buffer);
},
function chainExpression(node: ChainExpression, position, buffer) {
Expand Down Expand Up @@ -528,7 +530,8 @@ const bufferParsers: ((node: any, position: number, buffer: AstBuffer) => void)[
const parameters = (node.params = convertNodeList(node, scope, buffer[position + 3], buffer));
scope.addParameterVariables(
parameters.map(
parameter => parameter.declare('parameter', UNKNOWN_EXPRESSION) as ParameterVariable[]
parameter =>
parameter.declare('parameter', EMPTY_PATH, UNKNOWN_EXPRESSION) as ParameterVariable[]
),
parameters[parameters.length - 1] instanceof RestElement
);
Expand All @@ -546,7 +549,8 @@ const bufferParsers: ((node: any, position: number, buffer: AstBuffer) => void)[
const parameters = (node.params = convertNodeList(node, scope, buffer[position + 3], buffer));
scope.addParameterVariables(
parameters.map(
parameter => parameter.declare('parameter', UNKNOWN_EXPRESSION) as ParameterVariable[]
parameter =>
parameter.declare('parameter', EMPTY_PATH, UNKNOWN_EXPRESSION) as ParameterVariable[]
),
parameters[parameters.length - 1] instanceof RestElement
);
Expand Down
Loading

0 comments on commit d7062ef

Please sign in to comment.