Rework treeshaking algorithm

[lukastaegert]

This is a huge refactoring of several core aspects of rollups tree-shaking algorithm. Before going into implementation details, I will present you some things this pull request fixes or improves by showing you the output of the new/modified tests against the previous implementation (for the actual tests, please look at the code).

New test results

effect-in-for-loop

output with old implementation

const items = [{}, {}, {}];

for ( const a of items.children ) {
	a.foo = 'a';
}

let c;
for ( c of items.children ) {
	c.bar = 'c';
}

assert.deepEqual( items, [
	{ foo: 'a', bar: 'c', baz: 'e' },
	{ foo: 'a', bar: 'c', baz: 'e' },
	{ foo: 'a', bar: 'c', baz: 'e' }
]);

output with new implementation

const items = [{}, {}, {}];

for ( const a of items.children ) {
	a.foo = 'a';
}

let c;
for ( c of items.children ) {
	c.bar = 'c';
}

for ( e of items.children ) {
	e.baz = 'e';
}
var e;

assert.deepEqual( items, [
	{ foo: 'a', bar: 'c', baz: 'e' },
	{ foo: 'a', bar: 'c', baz: 'e' },
	{ foo: 'a', bar: 'c', baz: 'e' }
]);

I have added a new section to this test where the hoisted loop variable is defined AFTER the loop:

for ( e of items.children ) {
	e.baz = 'e';
}
var e;

As you can see, this code was completely ignored by the old implementation because the declaration of e was searched for in the scope before it had been added to the scope. This is now handled properly.

nested-tree-shaking

output with old implementation

function noEffects() {
	const foo = () => {};
	foo();
}

function withEffects() {
	noEffects();
	console.log('effect');
}

if (globalVar > 0) {
	console.log('effect');
	noEffects();
	withEffects();
}

output with new implementation

function withEffects() {
	console.log('effect');
}

if (globalVar > 0) {
	console.log('effect');
	withEffects();
}

Now tree-shaking occurs properly on all nesting levels, even functions inside functions inside functions will be tree-shaken properly.

side-effects-call-arguments

output with old implementation

var baz = 2;
assert.equal(baz, 3);

output with new implementation

function foo () {}

foo( globalFunction() );

var baz = 2;
foo( baz++ );

assert.equal(baz, 3);

FINALLY, side-effects in function call arguments are no longer ignored.

side-effects-delete

output with old implementation

var x = {foo: 'bar'};
delete x.foo;

var y = {foo: 'bar'};
delete y.foo;

delete globalVariable.foo;

export { x };

output with new implementation

var x = {foo: 'bar'};
delete x.foo;

delete globalVariable.foo;

export { x };

Deleting a field of a variable only needs to be a side-effect if the variable is actually included or global.

side-effects-pattern-assignment

output with old implementation

var a = {};
({x: a} = globalVar);

var b = {};
({b} = globalVar);

var e = {};
({x: e} = globalVar);
e.foo = 1;

var f = {};
({f} = globalVar);
f.foo = 1;

var {x: g} = globalVar;
g.foo = 1;

var {h} = globalVar;
h.foo = 1;

var i = {};
[i] = globalVar;

var k = {};
[,...k] = globalVar;

var m = {};
[m] = globalVar;
m.foo = 1;

var [n] = globalVar;
n.foo = 1;

var o = {};
[...o] = globalVar;
o.foo = 1;

var [...p] = globalVar;
p.foo = 1;

output with new implementation

var e = {};
({x: e} = globalVar);
e.foo = 1;

var f = {};
({f} = globalVar);
f.foo = 1;

var {x: g} = globalVar;
g.foo = 1;

var {h} = globalVar;
h.foo = 1;

var m = {};
[m] = globalVar;
m.foo = 1;

var [n] = globalVar;
n.foo = 1;

var o = {};
[...o] = globalVar;
o.foo = 1;

var [...p] = globalVar;
p.foo = 1;

Here, a, b, i and k were included unnecessarily. This is fixed now.

side-effects-pattern-defaults

output with old implementation

var c;
({ x: c = 1 } = {});

var d;
({ x: d = globalFunction() } = {});

var g;
[ g = 1 ] = [];

var h;
[ h = globalFunction() ] = [];

output with new implementation

var { x: b = globalFunction() } = {};

var d;
({ x: d = globalFunction() } = {});

var [ f = globalFunction() ] = [];

var h;
[ h = globalFunction() ] = [];

This was kind of broken:

• c and g were included unnecessarily, while (much worse)
• b and f were missing

This is fixed now.

side-effects-reassignment

output with old implementation

var foo = () => {};
foo = 3;
foo = 'foo';
foo = function () {
	console.log('effect');
};
foo = ['foo'];
foo = undefined;

var noEffect = () => {};
noEffect = function(a) {
	a = 'reassigned parameter';
};

var stillNoEffect = () => {};
stillNoEffect = noEffect;
stillNoEffect();

var effect = () => {};
effect = function() {
	console.log('effect');
};

var alsoEffect = () => {};
alsoEffect = effect;
alsoEffect();

output with new implementation

var effect = () => {};
effect = function() {
	console.log('effect');
};

var alsoEffect = () => {};
alsoEffect = effect;
alsoEffect();

This is probably the most important improvement to the tree-shaking algorithm. Previously any re-assignment with a non-trivial value was interpreted as a side-effect. Now only for mutations and calls, the algorithm looks at each assignment to an identifier and flags an effect if there actually is one.

Algorithmic/architecture changes

The main goal was to get rid of custom logic such as isUsedByBundle or assignToForLoopLeft that is hard to maintain and move logic to the individual nodes.

.included

• The flags used to indicate that something is needed (.activated, .ran, .shouldInclude) have been replaced by a single flag, .included. This flag is used by both nodes and other declarations and is usually set via .includeInBundle() for nodes and .includeDeclaration() for declarations. This distinction was necessary to be able to include side-effects of a default export without including the export itself.
  • A declaration (node) that is added via .includeInBundle() assumes that it is only included due to side-effects but not necessarily used. Therefore, it does not set its .included flag but includes children that .shouldBeIncluded() (see below)
  • A declaration that is added via .includeDeclaration() assumes that it is being used

Bundle

• The concept of dependent expressions has been removed
• Statements or module are no longer "run()"; I found it very hard to understand what running actually means anyway, at least the name did not help
• Instead, we repeatedly call Module.includeInBundle() on each module. If the return value is true for a module, then new nodes or declarations were added by this module. Repeat until all modules return false.

Module

Module.includeInBundle() includes each node that .shouldBeIncluded(), see below

Node

By default, Node inclusion works like this:

• A node .shouldBeIncluded() if it .hasEffects()
• An effect is either a side-effect or the fact that this node has already been included (which certainly has an effect on the bundle)
• .includeInBundle() will return true if this node has not been included yet or any child returns true; the return value signifies if something was added
• .includeInBundle() is cached via .isFullyIncluded() to make sure that only nodes are checked again that are still missing some child nodes

There are other new methods:

• .eachChild() no longer has special logic for rendering shorthand properties. This has been moved to Property.render
• .someChild() is a new helper method which accepts a callback predicate and bails out if the callback returns true for any child
• .assignExpression() and .hasEffectsWhenAssigned() handle the new assignment logic, see below
• .hasEffectsWhenMutated() handles the new mutation logic, see below

Assignments and Mutations

• declaration.possibleValues has been renamed to .assignedExpressions and now only ever receives either actual Nodes or the new UNKNOWN_ASSIGNMENT node. Previously, many different pseudo-values have been assiged here "for debugging purposes" which made the production code more difficult.
• Nodes can now have two new kinds of effects: .hasEffectsWhenAssigned() and .hasEffectsWhenMutated()
• A mutation is usually an added or removed property. In the future it is also conceivable that Object.freeze is treated as a mutation
• A mutation has an effect if a variable is global, can reference something global or is a parameter. Expressions that may return such a value have also effects when mutated (there is still room for further improvement)
• An assignment has an effect when assigning to a global variable or (possibly) a member expression but NOT to a function parameter.
• During bind, .assignExpression() assigns new nodes to declarations that can be checked for mutations etc.

And more that I can no longer remember :)
I hope you like it, feedback always welcome.

[lukastaegert]

Changes without whitespace adjustments:
https://github.com/rollup/rollup/pull/1582/files?w=1

[kzc]

@lukastaegert Nice work!

This PR fixes at least a couple of open issue (above).

[Rich-Harris]

Me reading this:

This is some seriously impressive work. I don't pretend to understand every part of the diff on first reading, though I expect it'll all become clear soon enough. Thank you so much!

[lukastaegert]

@Rich-Harris Thanks for the kind words 😃.
Unfortunately, there have been quite a few regressions, see #1586 😢
or, to put it more positively, #1586 adds quite a few new tests (and finally enables tree-shaking in (Do)WhileStatements 🎉).

So I hope you merge that one soon so I no longer have to bear the shame of having broken Rollup 😧

[Rich-Harris]

so I no longer have to bear the shame of having broken Rollup

we've all been there 😉 thanks for the speedy fixes — no harm, no foul