Skip to content

Commit

Permalink
[dart2js] inlining heuristics for generative constructor factory
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
rakudrama authored and [email protected] committed Jul 10, 2020
1 parent 9f8aaf6 commit cf7a63a
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 13 deletions.
198 changes: 188 additions & 10 deletions pkg/compiler/lib/src/ssa/builder_kernel.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6689,7 +6689,7 @@ class InlineWeeder extends ir.Visitor {
final bool enableUserAssertions;
final bool omitImplicitCasts;

final InlineData data = new InlineData();
final InlineData data = InlineData();
bool seenReturn = false;

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

// When handling a generative constructor factory, the super constructor calls
// are 'inlined', so tend to reuse the same parameters. [discountParameters]
// is true to avoid double-counting these parameters.
bool discountParameters = false;

InlineWeeder(
{this.enableUserAssertions: false, this.omitImplicitCasts: false});

Expand All @@ -6724,17 +6729,18 @@ class InlineWeeder extends ir.Visitor {
enableUserAssertions: enableUserAssertions,
omitImplicitCasts: omitImplicitCasts);
ir.FunctionNode node = getFunctionNode(elementMap, function);
node.accept(visitor);
if (function.isConstructor) {
visitor.data.isConstructor = true;
MemberDefinition definition = elementMap.getMemberDefinition(function);
visitor.skipReductiveNodes(() {
ir.Node node = definition.node;
if (node is ir.Constructor) {
visitor.visitList(node.initializers);
}
});
ir.Node node = definition.node;
if (node is ir.Constructor) {
visitor.skipReductiveNodes(() {
visitor.handleGenerativeConstructorFactory(node);
});
return visitor.data;
}
}
node.accept(visitor);
return visitor.data;
}

Expand All @@ -6752,9 +6758,9 @@ class InlineWeeder extends ir.Visitor {
countReductiveNode = oldCountReductiveNode;
}

void registerRegularNode() {
void registerRegularNode([int count = 1]) {
if (countRegularNode) {
data.regularNodeCount++;
data.regularNodeCount += count;
if (seenReturn) {
data.codeAfterReturn = true;
}
Expand Down Expand Up @@ -6986,6 +6992,7 @@ class InlineWeeder extends ir.Visitor {

@override
visitVariableGet(ir.VariableGet node) {
if (discountParameters && node.variable.parent is ir.FunctionNode) return;
registerRegularNode();
registerReductiveNode();
skipReductiveNodes(() => visit(node.promotedType));
Expand Down Expand Up @@ -7097,4 +7104,175 @@ class InlineWeeder extends ir.Visitor {
node.visitChildren(this);
data.hasIf = true;
}

void handleGenerativeConstructorFactory(ir.Constructor node) {
// Generative constructors are compiled to a factory constructor which
// contains inlined all the initializations up the inheritance chain and
// then call each of the constructor bodies down the inheritance chain.
ir.Constructor constructor = node;

Set<ir.Field> initializedFields = {};
bool hasCallToSomeConstructorBody = false;

inheritance_loop:
while (constructor != null) {
ir.Constructor superConstructor;
for (var initializer in constructor.initializers) {
if (initializer is ir.RedirectingInitializer) {
// Discount the size of the arguments by references that are
// pass-through.
// TODO(sra): Need to add size of defaulted arguments.
var discountParametersOld = discountParameters;
discountParameters = true;
initializer.arguments.accept(this);
discountParameters = discountParametersOld;
constructor = initializer.target;
continue inheritance_loop;
} else if (initializer is ir.SuperInitializer) {
superConstructor = initializer.target;
// Discount the size of the arguments by references that are
// pass-through.
// TODO(sra): Need to add size of defaulted arguments.
var discountParametersOld = discountParameters;
discountParameters = true;
initializer.arguments.accept(this);
discountParameters = discountParametersOld;
} else if (initializer is ir.FieldInitializer) {
initializedFields.add(initializer.field);
initializer.value.accept(this);
} else if (initializer is ir.AssertInitializer) {
if (enableUserAssertions) {
initializer.accept(this);
}
} else {
initializer.accept(this);
}
}

_handleFields(constructor.enclosingClass, initializedFields);

// There will be a call to the constructor's body, which might be empty
// and inlined away.
var function = constructor.function;
var body = function.body;
if (!isEmptyBody(body)) {
// All of the parameters are passed to the body.
int parameterCount = function.positionalParameters.length +
function.namedParameters.length +
function.typeParameters.length;

hasCallToSomeConstructorBody = true;
registerCall();
// A body call looks like "t.Body$(arguments);", i.e. an expression
// statement with an instance member call, but the receiver is not
// counted in the arguments. I'm guessing about 6 nodes for this.
registerRegularNode(
6 + parameterCount * INLINING_NODES_OUTSIDE_LOOP_ARG_FACTOR);

// We can't inline a generative constructor factory when one of the
// bodies rewrites the environment to put locals or parameters into a
// box. The box is created in the generative constructor factory since
// the box may be shared between closures in the initializer list and
// closures in the constructor body.
var bodyVisitor = InlineWeederBodyClosure();
body.accept(bodyVisitor);
if (bodyVisitor.tooDifficult) {
data.hasClosure = true;
}
}

if (superConstructor != null) {
// The class of the super-constructor may not be the supertype class. In
// this case, we must go up the class hierarchy until we reach the class
// containing the super-constructor.
ir.Supertype supertype = constructor.enclosingClass.supertype;
while (supertype.classNode != superConstructor.enclosingClass) {
_handleFields(supertype.classNode, initializedFields);
supertype = supertype.classNode.supertype;
}
}
constructor = superConstructor;
}

// In addition to the initializer expressions and body calls, there is an
// allocator call.
if (hasCallToSomeConstructorBody) {
// A temporary is requried so we have
//
// t=new ...;
// ...;
// use(t);
//
// I'm guessing it takes about 4 nodes to introduce the temporary and
// assign it.
registerRegularNode(4); // A temporary is requried.
}
// The initial field values are passed to the allocator.
registerRegularNode(
initializedFields.length * INLINING_NODES_OUTSIDE_LOOP_ARG_FACTOR);
}

void _handleFields(ir.Class cls, Set<ir.Field> initializedFields) {
for (ir.Field field in cls.fields) {
if (!field.isInstanceMember) continue;
ir.Expression initializer = field.initializer;
if (initializer == null ||
initializer is ir.ConstantExpression &&
initializer.constant is ir.PrimitiveConstant ||
initializer is ir.BasicLiteral) {
// Simple field initializers happen in the allocator, so do not
// contribute to the size of the generative constructor factory.
// TODO(sra): Use FieldInfo which tells us if the field is elided or
// initialized in the allocator.
continue;
}
if (!initializedFields.add(field)) continue;
initializer.accept(this);
}
// If [cls] is a mixin application, include fields from mixed in class.
if (cls.mixedInType != null) {
_handleFields(cls.mixedInType.classNode, initializedFields);
}
}

bool isEmptyBody(ir.Statement body) {
if (body is ir.EmptyStatement) return true;
if (body is ir.Block) return body.statements.every(isEmptyBody);
if (body is ir.AssertStatement && !enableUserAssertions) return true;
return false;
}
}

/// Visitor to detect environment-rewriting that prevents inlining
/// (e.g. closures).
class InlineWeederBodyClosure extends ir.Visitor<void> {
bool tooDifficult = false;

InlineWeederBodyClosure();

@override
defaultNode(ir.Node node) {
if (tooDifficult) return;
node.visitChildren(this);
}

@override
void visitFunctionExpression(ir.FunctionExpression node) {
tooDifficult = true;
}

@override
void visitFunctionDeclaration(ir.FunctionDeclaration node) {
tooDifficult = true;
}

@override
void visitFunctionNode(ir.FunctionNode node) {
assert(false);
if (node.asyncMarker != ir.AsyncMarker.Sync) {
tooDifficult = true;
return;
}
node.visitChildren(this);
}
}
2 changes: 1 addition & 1 deletion pkg/compiler/test/inlining/data/static_initializer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Tossing their heads in sprightly dance.

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

/*member: Foo.:[]*/
/*member: Foo.:[_var3:Foo]*/
class Foo {
int z = 99;
/*member: Foo.a:[_var3]*/
Expand Down
4 changes: 2 additions & 2 deletions pkg/compiler/test/inlining/data/type_variables.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ main() {
inlineTypeTests();
}

/*member: Mixin1.:[inlineTypeTests:Mixin1<int*>]*/
/*member: Mixin1.:closure*/
class Mixin1<S> {
var field = /*[]*/ (S s) => null;
}

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

/*member: _inlineTypeTests:[inlineTypeTests]*/
Expand Down

0 comments on commit cf7a63a

Please sign in to comment.