Skip to content

Commit

Permalink
(dart2js) add indirect dynamic calls to global inference and use it t…
Browse files Browse the repository at this point in the history
…o model operator==

On most apps, `==` is a very common invocation that adds lots of edges to
our global inference graph. It appears that for the most part we don't
learn much from it and that we save a considerable amount of space and
time durning global inference if we model them differently.

The amount of space varies a lot by application. Some large internal apps
show about 200K edges removed, other large apps show an incredibly large number
(1.3 M edges!)

Change-Id: I5168978f922c4d74adfae2de7fe69d56ff85ade0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/123980
Reviewed-by: Stephen Adams <sra@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
  • Loading branch information
sigmundch authored and commit-bot@chromium.org committed Nov 5, 2019
1 parent 9ba71b0 commit e65d95e
Show file tree
Hide file tree
Showing 6 changed files with 275 additions and 24 deletions.
103 changes: 90 additions & 13 deletions pkg/compiler/lib/src/inferrer/inferrer_engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,17 @@ abstract class InferrerEngine {
/// [NativeBehavior].
TypeInformation typeOfNativeBehavior(NativeBehavior nativeBehavior);

/// For a given selector, return a shared dynamic call site that will be used
/// to combine the results of multiple dynamic calls in the program via
/// [IndirectDynamicCallSiteTypeInformation].
///
/// This is used only for scalability reasons: if there are too many targets
/// and call sites, we may have a quadratic number of edges in the graph, so
/// we add a level of indirection to merge the information and keep the graph
/// smaller.
DynamicCallSiteTypeInformation typeOfSharedDynamicCall(
Selector selector, CallStructure structure);

bool returnsListElementType(Selector selector, AbstractValue mask);

bool returnsMapValueType(Selector selector, AbstractValue mask);
Expand Down Expand Up @@ -397,11 +408,14 @@ class InferrerEngineImpl extends InferrerEngine {
void updateSelectorInMember(MemberEntity owner, CallType callType,
ir.Node node, Selector selector, AbstractValue mask) {
KernelGlobalTypeInferenceElementData data = dataOfMember(owner);
assert(validCallType(callType, node));
assert(validCallType(callType, node, selector));
switch (callType) {
case CallType.access:
data.setTypeMask(node, mask);
break;
case CallType.indirectAccess:
// indirect access is not diretly recorded in the result data.

This comment has been minimized.

Copy link
@robbecker-wf

robbecker-wf Nov 5, 2019

typo: should be directly

break;
case CallType.forIn:
if (selector == Selectors.iterator) {
data.setIteratorTypeMask(node, mask);
Expand Down Expand Up @@ -1142,19 +1156,40 @@ class InferrerEngineImpl extends InferrerEngine {
updateSideEffects(sideEffectsBuilder, selector, callee);
});

CallSiteTypeInformation info = new DynamicCallSiteTypeInformation(
abstractValueDomain,
types.currentMember,
callType,
node,
caller,
selector,
mask,
receiverType,
arguments,
inLoop,
isConditional);
CallSiteTypeInformation info;

// We force using indirection for `==` because it is a very common dynamic
// call site on many apps.
// TODO(sigmund): it would be even better if we could automatically detect
// when dynamic calls are growing too big and add the indirection at that
// point.
if (selector.name == '==') {
info = new IndirectDynamicCallSiteTypeInformation(
abstractValueDomain,
types.currentMember,
node,
typeOfSharedDynamicCall(selector, CallStructure.ONE_ARG),
caller,
selector,
mask,
receiverType,
arguments,
inLoop,
isConditional);
} else {
info = new DynamicCallSiteTypeInformation(
abstractValueDomain,
types.currentMember,
callType,
node,
caller,
selector,
mask,
receiverType,
arguments,
inLoop,
isConditional);
}
info.addToGraph(this);
types.allocatedCalls.add(info);
return info;
Expand Down Expand Up @@ -1282,6 +1317,48 @@ class InferrerEngineImpl extends InferrerEngine {
}
}

/// Indirect calls share the same dynamic call site information node. This
/// cache holds that shared dynamic call node for a given selector.
Map<Selector, DynamicCallSiteTypeInformation> _sharedCalls = {};

@override
DynamicCallSiteTypeInformation typeOfSharedDynamicCall(
Selector selector, CallStructure structure) {
DynamicCallSiteTypeInformation info = _sharedCalls[selector];
if (info != null) return info;

TypeInformation receiverType =
new IndirectParameterTypeInformation(abstractValueDomain, 'receiver');
List<TypeInformation> positional = [];
for (int i = 0; i < structure.positionalArgumentCount; i++) {
positional
.add(new IndirectParameterTypeInformation(abstractValueDomain, '$i'));
}
Map<String, TypeInformation> named = {};
if (structure.namedArgumentCount > 0) {
for (var name in structure.namedArguments) {
named[name] =
new IndirectParameterTypeInformation(abstractValueDomain, name);
}
}

info = _sharedCalls[selector] = new DynamicCallSiteTypeInformation(
abstractValueDomain,
null,
CallType.indirectAccess,
null,
null,
selector,
null,
receiverType,
ArgumentsTypes(positional, named),
false,
false);
info.addToGraph(this);
types.allocatedCalls.add(info);
return info;
}

@override
bool canFieldBeUsedForGlobalOptimizations(FieldEntity element) {
if (closedWorld.backendUsage.isFieldUsedByBackend(element)) {
Expand Down
14 changes: 14 additions & 0 deletions pkg/compiler/lib/src/inferrer/node_tracer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ abstract class TracerVisitor implements TypeInformationVisitor {
}
}

@override
visitIndirectDynamicCallSiteTypeInformation(
IndirectDynamicCallSiteTypeInformation info) {
if (info.dynamicCall == currentUser) {
addNewEscapeInformation(info);
}
}

void analyzeStoredIntoList(ListTypeInformation list) {
inferrer.analyzeListAndEnqueue(list);
if (list.bailedOut) {
Expand Down Expand Up @@ -547,4 +555,10 @@ abstract class TracerVisitor implements TypeInformationVisitor {
}
addNewEscapeInformation(info);
}

@override
void visitIndirectParameterTypeInformation(
IndirectParameterTypeInformation info) {
addNewEscapeInformation(info);
}
}
12 changes: 12 additions & 0 deletions pkg/compiler/lib/src/inferrer/type_graph_dump.dart
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,12 @@ class _GraphGenerator extends TypeInformationVisitor {
handleCall(info, 'StaticCallSite', {});
}

@override
void visitIndirectDynamicCallSiteTypeInformation(
IndirectDynamicCallSiteTypeInformation info) {
handleCall(info, 'IndirectDynamicCallSite', {});
}

@override
void visitDynamicCallSiteTypeInformation(
DynamicCallSiteTypeInformation info) {
Expand All @@ -426,6 +432,12 @@ class _GraphGenerator extends TypeInformationVisitor {
addNode(info, 'Parameter ${info.debugName}');
}

@override
void visitIndirectParameterTypeInformation(
IndirectParameterTypeInformation info) {
addNode(info, 'IndirectParameter ${info.debugName}');
}

@override
void visitClosureTypeInformation(ClosureTypeInformation info) {
String text = shorten('${info.debugName}');
Expand Down
3 changes: 1 addition & 2 deletions pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ class TypeGraphInferrer implements TypesInferrer {
returnType != null &&
abstractValueDomain.isEmpty(returnType).isDefinitelyTrue;

bool isCalledOnce =
typeInformation.isCalledOnce(); //isMemberCalledOnce(member);
bool isCalledOnce = typeInformation.isCalledOnce();

memberResults[member] = new GlobalTypeInferenceMemberResultImpl(
data, returnType, type,
Expand Down
Loading

1 comment on commit e65d95e

@aaronlademann-wf
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome

Please sign in to comment.