Skip to content

Commit cf7a63a

Browse files
rakudramacommit-bot@chromium.org
authored andcommitted
[dart2js] inlining heuristics for generative constructor factory
Account for the size of the a generative constructor factory. The factory evaluates all the initializers up the inheritance/mixin chain, allocates the object, and then calls all the constructors down the inheritance chain. All this is missed by the old code looking at the immediate constructor body, sometimes causing extremely large constructors to be inlined. Change-Id: I8803b0f4b4155dec0f7878fd5527c1dd507b9c80 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153560 Commit-Queue: Stephen Adams <[email protected]> Reviewed-by: Mayank Patke <[email protected]>
1 parent 9f8aaf6 commit cf7a63a

File tree

3 files changed

+191
-13
lines changed

3 files changed

+191
-13
lines changed

pkg/compiler/lib/src/ssa/builder_kernel.dart

Lines changed: 188 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6689,7 +6689,7 @@ class InlineWeeder extends ir.Visitor {
66896689
final bool enableUserAssertions;
66906690
final bool omitImplicitCasts;
66916691

6692-
final InlineData data = new InlineData();
6692+
final InlineData data = InlineData();
66936693
bool seenReturn = false;
66946694

66956695
/// Whether node-count is collector to determine if a function can be
@@ -6714,6 +6714,11 @@ class InlineWeeder extends ir.Visitor {
67146714
// TODO(25231): Make larger string constants eligible by sharing references.
67156715
bool countReductiveNode = true;
67166716

6717+
// When handling a generative constructor factory, the super constructor calls
6718+
// are 'inlined', so tend to reuse the same parameters. [discountParameters]
6719+
// is true to avoid double-counting these parameters.
6720+
bool discountParameters = false;
6721+
67176722
InlineWeeder(
67186723
{this.enableUserAssertions: false, this.omitImplicitCasts: false});
67196724

@@ -6724,17 +6729,18 @@ class InlineWeeder extends ir.Visitor {
67246729
enableUserAssertions: enableUserAssertions,
67256730
omitImplicitCasts: omitImplicitCasts);
67266731
ir.FunctionNode node = getFunctionNode(elementMap, function);
6727-
node.accept(visitor);
67286732
if (function.isConstructor) {
67296733
visitor.data.isConstructor = true;
67306734
MemberDefinition definition = elementMap.getMemberDefinition(function);
6731-
visitor.skipReductiveNodes(() {
6732-
ir.Node node = definition.node;
6733-
if (node is ir.Constructor) {
6734-
visitor.visitList(node.initializers);
6735-
}
6736-
});
6735+
ir.Node node = definition.node;
6736+
if (node is ir.Constructor) {
6737+
visitor.skipReductiveNodes(() {
6738+
visitor.handleGenerativeConstructorFactory(node);
6739+
});
6740+
return visitor.data;
6741+
}
67376742
}
6743+
node.accept(visitor);
67386744
return visitor.data;
67396745
}
67406746

@@ -6752,9 +6758,9 @@ class InlineWeeder extends ir.Visitor {
67526758
countReductiveNode = oldCountReductiveNode;
67536759
}
67546760

6755-
void registerRegularNode() {
6761+
void registerRegularNode([int count = 1]) {
67566762
if (countRegularNode) {
6757-
data.regularNodeCount++;
6763+
data.regularNodeCount += count;
67586764
if (seenReturn) {
67596765
data.codeAfterReturn = true;
67606766
}
@@ -6986,6 +6992,7 @@ class InlineWeeder extends ir.Visitor {
69866992

69876993
@override
69886994
visitVariableGet(ir.VariableGet node) {
6995+
if (discountParameters && node.variable.parent is ir.FunctionNode) return;
69896996
registerRegularNode();
69906997
registerReductiveNode();
69916998
skipReductiveNodes(() => visit(node.promotedType));
@@ -7097,4 +7104,175 @@ class InlineWeeder extends ir.Visitor {
70977104
node.visitChildren(this);
70987105
data.hasIf = true;
70997106
}
7107+
7108+
void handleGenerativeConstructorFactory(ir.Constructor node) {
7109+
// Generative constructors are compiled to a factory constructor which
7110+
// contains inlined all the initializations up the inheritance chain and
7111+
// then call each of the constructor bodies down the inheritance chain.
7112+
ir.Constructor constructor = node;
7113+
7114+
Set<ir.Field> initializedFields = {};
7115+
bool hasCallToSomeConstructorBody = false;
7116+
7117+
inheritance_loop:
7118+
while (constructor != null) {
7119+
ir.Constructor superConstructor;
7120+
for (var initializer in constructor.initializers) {
7121+
if (initializer is ir.RedirectingInitializer) {
7122+
// Discount the size of the arguments by references that are
7123+
// pass-through.
7124+
// TODO(sra): Need to add size of defaulted arguments.
7125+
var discountParametersOld = discountParameters;
7126+
discountParameters = true;
7127+
initializer.arguments.accept(this);
7128+
discountParameters = discountParametersOld;
7129+
constructor = initializer.target;
7130+
continue inheritance_loop;
7131+
} else if (initializer is ir.SuperInitializer) {
7132+
superConstructor = initializer.target;
7133+
// Discount the size of the arguments by references that are
7134+
// pass-through.
7135+
// TODO(sra): Need to add size of defaulted arguments.
7136+
var discountParametersOld = discountParameters;
7137+
discountParameters = true;
7138+
initializer.arguments.accept(this);
7139+
discountParameters = discountParametersOld;
7140+
} else if (initializer is ir.FieldInitializer) {
7141+
initializedFields.add(initializer.field);
7142+
initializer.value.accept(this);
7143+
} else if (initializer is ir.AssertInitializer) {
7144+
if (enableUserAssertions) {
7145+
initializer.accept(this);
7146+
}
7147+
} else {
7148+
initializer.accept(this);
7149+
}
7150+
}
7151+
7152+
_handleFields(constructor.enclosingClass, initializedFields);
7153+
7154+
// There will be a call to the constructor's body, which might be empty
7155+
// and inlined away.
7156+
var function = constructor.function;
7157+
var body = function.body;
7158+
if (!isEmptyBody(body)) {
7159+
// All of the parameters are passed to the body.
7160+
int parameterCount = function.positionalParameters.length +
7161+
function.namedParameters.length +
7162+
function.typeParameters.length;
7163+
7164+
hasCallToSomeConstructorBody = true;
7165+
registerCall();
7166+
// A body call looks like "t.Body$(arguments);", i.e. an expression
7167+
// statement with an instance member call, but the receiver is not
7168+
// counted in the arguments. I'm guessing about 6 nodes for this.
7169+
registerRegularNode(
7170+
6 + parameterCount * INLINING_NODES_OUTSIDE_LOOP_ARG_FACTOR);
7171+
7172+
// We can't inline a generative constructor factory when one of the
7173+
// bodies rewrites the environment to put locals or parameters into a
7174+
// box. The box is created in the generative constructor factory since
7175+
// the box may be shared between closures in the initializer list and
7176+
// closures in the constructor body.
7177+
var bodyVisitor = InlineWeederBodyClosure();
7178+
body.accept(bodyVisitor);
7179+
if (bodyVisitor.tooDifficult) {
7180+
data.hasClosure = true;
7181+
}
7182+
}
7183+
7184+
if (superConstructor != null) {
7185+
// The class of the super-constructor may not be the supertype class. In
7186+
// this case, we must go up the class hierarchy until we reach the class
7187+
// containing the super-constructor.
7188+
ir.Supertype supertype = constructor.enclosingClass.supertype;
7189+
while (supertype.classNode != superConstructor.enclosingClass) {
7190+
_handleFields(supertype.classNode, initializedFields);
7191+
supertype = supertype.classNode.supertype;
7192+
}
7193+
}
7194+
constructor = superConstructor;
7195+
}
7196+
7197+
// In addition to the initializer expressions and body calls, there is an
7198+
// allocator call.
7199+
if (hasCallToSomeConstructorBody) {
7200+
// A temporary is requried so we have
7201+
//
7202+
// t=new ...;
7203+
// ...;
7204+
// use(t);
7205+
//
7206+
// I'm guessing it takes about 4 nodes to introduce the temporary and
7207+
// assign it.
7208+
registerRegularNode(4); // A temporary is requried.
7209+
}
7210+
// The initial field values are passed to the allocator.
7211+
registerRegularNode(
7212+
initializedFields.length * INLINING_NODES_OUTSIDE_LOOP_ARG_FACTOR);
7213+
}
7214+
7215+
void _handleFields(ir.Class cls, Set<ir.Field> initializedFields) {
7216+
for (ir.Field field in cls.fields) {
7217+
if (!field.isInstanceMember) continue;
7218+
ir.Expression initializer = field.initializer;
7219+
if (initializer == null ||
7220+
initializer is ir.ConstantExpression &&
7221+
initializer.constant is ir.PrimitiveConstant ||
7222+
initializer is ir.BasicLiteral) {
7223+
// Simple field initializers happen in the allocator, so do not
7224+
// contribute to the size of the generative constructor factory.
7225+
// TODO(sra): Use FieldInfo which tells us if the field is elided or
7226+
// initialized in the allocator.
7227+
continue;
7228+
}
7229+
if (!initializedFields.add(field)) continue;
7230+
initializer.accept(this);
7231+
}
7232+
// If [cls] is a mixin application, include fields from mixed in class.
7233+
if (cls.mixedInType != null) {
7234+
_handleFields(cls.mixedInType.classNode, initializedFields);
7235+
}
7236+
}
7237+
7238+
bool isEmptyBody(ir.Statement body) {
7239+
if (body is ir.EmptyStatement) return true;
7240+
if (body is ir.Block) return body.statements.every(isEmptyBody);
7241+
if (body is ir.AssertStatement && !enableUserAssertions) return true;
7242+
return false;
7243+
}
7244+
}
7245+
7246+
/// Visitor to detect environment-rewriting that prevents inlining
7247+
/// (e.g. closures).
7248+
class InlineWeederBodyClosure extends ir.Visitor<void> {
7249+
bool tooDifficult = false;
7250+
7251+
InlineWeederBodyClosure();
7252+
7253+
@override
7254+
defaultNode(ir.Node node) {
7255+
if (tooDifficult) return;
7256+
node.visitChildren(this);
7257+
}
7258+
7259+
@override
7260+
void visitFunctionExpression(ir.FunctionExpression node) {
7261+
tooDifficult = true;
7262+
}
7263+
7264+
@override
7265+
void visitFunctionDeclaration(ir.FunctionDeclaration node) {
7266+
tooDifficult = true;
7267+
}
7268+
7269+
@override
7270+
void visitFunctionNode(ir.FunctionNode node) {
7271+
assert(false);
7272+
if (node.asyncMarker != ir.AsyncMarker.Sync) {
7273+
tooDifficult = true;
7274+
return;
7275+
}
7276+
node.visitChildren(this);
7277+
}
71007278
}

pkg/compiler/test/inlining/data/static_initializer.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ Tossing their heads in sprightly dance.
4040

4141
var _var3 = <int>[Foo().a, (Foo()..a).a];
4242

43-
/*member: Foo.:[]*/
43+
/*member: Foo.:[_var3:Foo]*/
4444
class Foo {
4545
int z = 99;
4646
/*member: Foo.a:[_var3]*/

pkg/compiler/test/inlining/data/type_variables.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ main() {
99
inlineTypeTests();
1010
}
1111

12-
/*member: Mixin1.:[inlineTypeTests:Mixin1<int*>]*/
12+
/*member: Mixin1.:closure*/
1313
class Mixin1<S> {
1414
var field = /*[]*/ (S s) => null;
1515
}
1616

17-
/*member: Class1.:[inlineTypeTests:Class1<int*>]*/
17+
/*member: Class1.:closure*/
1818
class Class1<T> extends Object with Mixin1<T> {}
1919

2020
/*member: _inlineTypeTests:[inlineTypeTests]*/

0 commit comments

Comments
 (0)