From e65d95e8c869a6ef26a5fdb6951267ea06c3e6fe Mon Sep 17 00:00:00 2001 From: Sigmund Cherem Date: Tue, 5 Nov 2019 18:17:13 +0000 Subject: [PATCH] (dart2js) add indirect dynamic calls to global inference and use it to 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 Commit-Queue: Sigmund Cherem --- .../lib/src/inferrer/inferrer_engine.dart | 103 +++++++++-- .../lib/src/inferrer/node_tracer.dart | 14 ++ .../lib/src/inferrer/type_graph_dump.dart | 12 ++ .../lib/src/inferrer/type_graph_inferrer.dart | 3 +- .../lib/src/inferrer/type_graph_nodes.dart | 164 +++++++++++++++++- .../dart2js/inference/data/general.dart | 3 +- 6 files changed, 275 insertions(+), 24 deletions(-) diff --git a/pkg/compiler/lib/src/inferrer/inferrer_engine.dart b/pkg/compiler/lib/src/inferrer/inferrer_engine.dart index bf214c119066..a4accabea2e8 100644 --- a/pkg/compiler/lib/src/inferrer/inferrer_engine.dart +++ b/pkg/compiler/lib/src/inferrer/inferrer_engine.dart @@ -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); @@ -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. + break; case CallType.forIn: if (selector == Selectors.iterator) { data.setIteratorTypeMask(node, mask); @@ -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; @@ -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 _sharedCalls = {}; + + @override + DynamicCallSiteTypeInformation typeOfSharedDynamicCall( + Selector selector, CallStructure structure) { + DynamicCallSiteTypeInformation info = _sharedCalls[selector]; + if (info != null) return info; + + TypeInformation receiverType = + new IndirectParameterTypeInformation(abstractValueDomain, 'receiver'); + List positional = []; + for (int i = 0; i < structure.positionalArgumentCount; i++) { + positional + .add(new IndirectParameterTypeInformation(abstractValueDomain, '$i')); + } + Map 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)) { diff --git a/pkg/compiler/lib/src/inferrer/node_tracer.dart b/pkg/compiler/lib/src/inferrer/node_tracer.dart index 987cb11dd54e..4d4a4bd865d0 100644 --- a/pkg/compiler/lib/src/inferrer/node_tracer.dart +++ b/pkg/compiler/lib/src/inferrer/node_tracer.dart @@ -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) { @@ -547,4 +555,10 @@ abstract class TracerVisitor implements TypeInformationVisitor { } addNewEscapeInformation(info); } + + @override + void visitIndirectParameterTypeInformation( + IndirectParameterTypeInformation info) { + addNewEscapeInformation(info); + } } diff --git a/pkg/compiler/lib/src/inferrer/type_graph_dump.dart b/pkg/compiler/lib/src/inferrer/type_graph_dump.dart index 70d3fa081bae..a4ed3e336969 100644 --- a/pkg/compiler/lib/src/inferrer/type_graph_dump.dart +++ b/pkg/compiler/lib/src/inferrer/type_graph_dump.dart @@ -410,6 +410,12 @@ class _GraphGenerator extends TypeInformationVisitor { handleCall(info, 'StaticCallSite', {}); } + @override + void visitIndirectDynamicCallSiteTypeInformation( + IndirectDynamicCallSiteTypeInformation info) { + handleCall(info, 'IndirectDynamicCallSite', {}); + } + @override void visitDynamicCallSiteTypeInformation( DynamicCallSiteTypeInformation info) { @@ -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}'); diff --git a/pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart b/pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart index 3a4195137e67..a1e7c153b162 100644 --- a/pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart +++ b/pkg/compiler/lib/src/inferrer/type_graph_inferrer.dart @@ -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, diff --git a/pkg/compiler/lib/src/inferrer/type_graph_nodes.dart b/pkg/compiler/lib/src/inferrer/type_graph_nodes.dart index f3a949d93589..f674734c23a6 100644 --- a/pkg/compiler/lib/src/inferrer/type_graph_nodes.dart +++ b/pkg/compiler/lib/src/inferrer/type_graph_nodes.dart @@ -377,6 +377,10 @@ abstract class MemberTypeInformation extends ElementTypeInformation // cleanup has been called. bool _isCalledOnce = null; + /// Whether this member is invoked via indirect dynamic calls. In that case + /// the exact number of call sites cannot be computed precisely. + bool _calledIndirectly = false; + /// This map contains the callers of [element]. It stores all unique call /// sites to enable counting the global number of call sites of [element]. /// @@ -427,6 +431,7 @@ abstract class MemberTypeInformation extends ElementTypeInformation } bool _computeIsCalledOnce() { + if (_calledIndirectly) return false; if (_callers == null) return false; int count = 0; for (var set in _callers.values) { @@ -872,15 +877,40 @@ class ParameterTypeInformation extends ElementTypeInformation { } } +/// A synthetic parameter used to model the entry points to a +/// [IndirectDynamicCallSiteTypeInformation]. +class IndirectParameterTypeInformation extends TypeInformation { + final String debugName; + + IndirectParameterTypeInformation( + AbstractValueDomain abstractValueDomain, this.debugName) + : super(abstractValueDomain.emptyType, null); + + @override + AbstractValue computeType(InferrerEngine inferrer) => + inferrer.types.computeTypeMask(assignments); + + @override + accept(TypeInformationVisitor visitor) { + return visitor.visitIndirectParameterTypeInformation(this); + } + + @override + String toString() => 'IndirectParameter $debugName $type'; +} + enum CallType { access, + indirectAccess, forIn, } -bool validCallType(CallType callType, Object call) { +bool validCallType(CallType callType, Object call, Selector selector) { switch (callType) { case CallType.access: return call is ir.Node; + case CallType.indirectAccess: + return call == null && selector.name == '=='; case CallType.forIn: return call is ir.ForInStatement; } @@ -914,7 +944,7 @@ abstract class CallSiteTypeInformation extends TypeInformation this.arguments, this.inLoop) : super.noAssignments(abstractValueDomain.emptyType, context) { - assert(_call is ir.Node); + assert(_call is ir.Node || (_call == null && selector.name == '==')); } @override @@ -1011,6 +1041,109 @@ class StaticCallSiteTypeInformation extends CallSiteTypeInformation { } } +/// A call modeled with a level of indirection. +/// +/// This kind of call is artificial and only exists to address scalability +/// limitations of the inference algorithm. Any virtual, interface, or dynamic +/// call is normally modeled via [DynamicCallSiteTypeInformation]. The main +/// scalability concern of those calls is that we may get a quadratic number of +/// edges to model all the argument and return values (an edge per dynamic call +/// and target pair). Adding a level of indirection helps in that all these +/// calls are funneled through a single [DynamicCallSiteTypeInformation] node, +/// this in turn reduces the edges to be linear (an edge per indiret call to the +/// dynamic call node, and an edge from the call to each target). +class IndirectDynamicCallSiteTypeInformation extends CallSiteTypeInformation { + final DynamicCallSiteTypeInformation dynamicCall; + final bool isConditional; + final TypeInformation receiver; + + IndirectDynamicCallSiteTypeInformation( + AbstractValueDomain abstractValueDomain, + MemberTypeInformation context, + Object call, + this.dynamicCall, + MemberEntity enclosing, + Selector selector, + AbstractValue mask, + this.receiver, + ArgumentsTypes arguments, + bool inLoop, + this.isConditional) + : super(abstractValueDomain, context, call, enclosing, selector, mask, + arguments, inLoop); + + @override + void addToGraph(InferrerEngine inferrer) { + receiver.addUser(this); + dynamicCall.receiver.addAssignment(receiver); + List positional = arguments.positional; + for (int i = 0; i < positional.length; i++) { + positional[i].addUser(this); + dynamicCall.arguments.positional[i].addAssignment(positional[i]); + } + arguments.named.forEach((name, namedInfo) { + dynamicCall.arguments.named[name].addAssignment(namedInfo); + }); + dynamicCall.addUser(this); + } + + @override + AbstractValue computeType(InferrerEngine inferrer) { + AbstractValue typeMask = _computeTypedSelector(inferrer); + inferrer.updateSelectorInMember( + caller, CallType.access, _call, selector, typeMask); + + AbstractValue result = dynamicCall.computeType(inferrer); + AbstractValueDomain abstractValueDomain = + inferrer.closedWorld.abstractValueDomain; + if (isConditional && + abstractValueDomain.isNull(receiver.type).isPotentiallyTrue) { + // Conditional calls like `a?.b` may be null if the receiver is null. + result = abstractValueDomain.includeNull(result); + } + return result; + } + + AbstractValue _computeTypedSelector(InferrerEngine inferrer) { + AbstractValue receiverType = receiver.type; + if (mask == receiverType) return mask; + return receiverType == inferrer.abstractValueDomain.dynamicType + ? null + : receiverType; + } + + @override + void giveUp(InferrerEngine inferrer, {bool clearAssignments: true}) { + if (!abandonInferencing) { + inferrer.updateSelectorInMember( + caller, CallType.access, _call, selector, mask); + } + super.giveUp(inferrer, clearAssignments: clearAssignments); + } + + @override + Iterable get callees => dynamicCall.callees; + + @override + accept(TypeInformationVisitor visitor) { + return visitor.visitIndirectDynamicCallSiteTypeInformation(this); + } + + @override + bool hasStableType(InferrerEngine inferrer) => + dynamicCall.hasStableType(inferrer); + + @override + void removeAndClearReferences(InferrerEngine inferrer) { + dynamicCall.removeUser(this); + receiver.removeUser(this); + if (arguments != null) { + arguments.forEach((info) => info.removeUser(this)); + } + super.removeAndClearReferences(inferrer); + } +} + class DynamicCallSiteTypeInformation extends CallSiteTypeInformation { final CallType _callType; final TypeInformation receiver; @@ -1034,7 +1167,21 @@ class DynamicCallSiteTypeInformation extends CallSiteTypeInformation { this.isConditional) : super(abstractValueDomain, context, call, enclosing, selector, mask, arguments, inLoop) { - assert(validCallType(_callType, _call)); + assert(validCallType(_callType, _call, selector)); + } + + void _addCall(MemberTypeInformation callee) { + if (_callType == CallType.indirectAccess) { + callee._calledIndirectly = true; + } else { + callee.addCall(caller, _call); + } + } + + void _removeCall(MemberTypeInformation callee) { + if (_callType != CallType.indirectAccess) { + callee.removeCall(caller, _call); + } } @override @@ -1051,7 +1198,7 @@ class DynamicCallSiteTypeInformation extends CallSiteTypeInformation { for (MemberEntity element in _concreteTargets) { MemberTypeInformation callee = inferrer.types.getInferredTypeOfMember(element); - callee.addCall(caller, _call); + _addCall(callee); callee.addUser(this); inferrer.updateParameterAssignments( this, element, arguments, selector, typeMask, @@ -1072,7 +1219,6 @@ class DynamicCallSiteTypeInformation extends CallSiteTypeInformation { AbstractValue computeTypedSelector(InferrerEngine inferrer) { AbstractValue receiverType = receiver.type; - if (mask != receiverType) { return receiverType == inferrer.abstractValueDomain.dynamicType ? null @@ -1216,7 +1362,7 @@ class DynamicCallSiteTypeInformation extends CallSiteTypeInformation { .forEach((MemberEntity element) { MemberTypeInformation callee = inferrer.types.getInferredTypeOfMember(element); - callee.addCall(caller, _call); + _addCall(callee); callee.addUser(this); inferrer.updateParameterAssignments( this, element, arguments, selector, typeMask, @@ -1229,7 +1375,7 @@ class DynamicCallSiteTypeInformation extends CallSiteTypeInformation { .forEach((MemberEntity element) { MemberTypeInformation callee = inferrer.types.getInferredTypeOfMember(element); - callee.removeCall(caller, _call); + _removeCall(callee); callee.removeUser(this); inferrer.updateParameterAssignments( this, element, arguments, selector, typeMask, @@ -2111,8 +2257,12 @@ abstract class TypeInformationVisitor { T visitClosureCallSiteTypeInformation(ClosureCallSiteTypeInformation info); T visitStaticCallSiteTypeInformation(StaticCallSiteTypeInformation info); T visitDynamicCallSiteTypeInformation(DynamicCallSiteTypeInformation info); + T visitIndirectDynamicCallSiteTypeInformation( + IndirectDynamicCallSiteTypeInformation info); T visitMemberTypeInformation(MemberTypeInformation info); T visitParameterTypeInformation(ParameterTypeInformation info); + T visitIndirectParameterTypeInformation( + IndirectParameterTypeInformation info); T visitClosureTypeInformation(ClosureTypeInformation info); T visitAwaitTypeInformation(AwaitTypeInformation info); T visitYieldTypeInformation(YieldTypeInformation info); diff --git a/tests/compiler/dart2js/inference/data/general.dart b/tests/compiler/dart2js/inference/data/general.dart index 678911c11fb3..beb067daf3e6 100644 --- a/tests/compiler/dart2js/inference/data/general.dart +++ b/tests/compiler/dart2js/inference/data/general.dart @@ -675,8 +675,7 @@ class A { A.generative(); /*member: A.==:[exact=JSBool]*/ - operator ==(/*Union([exact=JSString], [exact=JSUInt31])*/ other) => - 42 as dynamic; + operator ==(/*[null|subclass=Object]*/ other) => 42 as dynamic; /*member: A.myField:[exact=JSUInt31]*/ get myField => 42;