Skip to content

Commit

Permalink
[dart2js] Speed up slow operation in closure tracing.
Browse files Browse the repository at this point in the history 
Previously both updated locations were using an expensive 'any' operation to see if one of the concrete targets was the 'currentUser'. This required iterating every target, which there could be many of for dynamic call sites. Looking for the member itself with a 'contains' operation on the concrete target list is much faster since the concrete target lists are backed by Setlets.

Locally this change lead to a ~15% improvement in runtime. The time spent tracing closures in a particular large app went from ~30s to ~2s.

Change-Id: Id9cd1cb9a8d8f0990893c827b9a3b49003180d8e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268166
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
  • Loading branch information
@natebiggs
natebiggs authored and Commit Queue committed Nov 7, 2022
1 parent e9709fe commit 5829f93
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 182 deletions.
15 changes: 9 additions & 6 deletions pkg/compiler/lib/src/inferrer/closure_tracer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,6 @@ class ClosureTracerVisitor extends TracerVisitor {
}
}

bool _checkIfCurrentUser(MemberEntity element) =>
inferrer.types.getInferredTypeOfMember(element) == currentUser;

bool _checkIfFunctionApply(MemberEntity element) {
return inferrer.closedWorld.commonElements.isFunctionApplyMethod(element);
}
Expand All @@ -118,17 +115,23 @@ class ClosureTracerVisitor extends TracerVisitor {
visitDynamicCallSiteTypeInformation(DynamicCallSiteTypeInformation info) {
super.visitDynamicCallSiteTypeInformation(info);
final selector = info.selector!;
final user = currentUser;
if (selector.isCall) {
if (info.arguments!.contains(currentUser)) {
if (info.arguments!.contains(user)) {
if (info.hasClosureCallTargets ||
info.concreteTargets.any((element) => !element.isFunction)) {
bailout('Passed to a closure');
}
if (info.concreteTargets.any(_checkIfFunctionApply)) {
_tagAsFunctionApplyTarget("dynamic call");
}
} else if (info.concreteTargets.any(_checkIfCurrentUser)) {
_registerCallForLaterAnalysis(info);
} else {
if (user is MemberTypeInformation) {
final currentUserMember = user.member;
if (info.concreteTargets.contains(currentUserMember)) {
_registerCallForLaterAnalysis(info);
}
}
}
} else if (selector.isGetter && selector.memberName == Names.call) {
// We are potentially tearing off ourself here
Expand Down
163 changes: 84 additions & 79 deletions pkg/compiler/lib/src/inferrer/engine.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,95 +316,98 @@ class InferrerEngine {
_buildWorkQueue();
metrics.refine1.measure(_refine);

// Try to infer element types of lists and compute their escape information.
types.allocatedLists.values.forEach((ListTypeInformation info) {
analyzeListAndEnqueue(info);
});

// Try to infer element types of sets and compute their escape information.
types.allocatedSets.values.forEach((SetTypeInformation info) {
analyzeSetAndEnqueue(info);
});
metrics.trace.measure(() {
// Try to infer element types of lists and compute their escape information.
types.allocatedLists.values.forEach((ListTypeInformation info) {
analyzeListAndEnqueue(info);
});

// Try to infer the key and value types for maps and compute the values'
// escape information.
types.allocatedMaps.values.forEach((MapTypeInformation info) {
analyzeMapAndEnqueue(info);
});
// Try to infer element types of sets and compute their escape information.
types.allocatedSets.values.forEach((SetTypeInformation info) {
analyzeSetAndEnqueue(info);
});

Set<FunctionEntity> bailedOutOn = Set<FunctionEntity>();
// Try to infer the key and value types for maps and compute the values'
// escape information.
types.allocatedMaps.values.forEach((MapTypeInformation info) {
analyzeMapAndEnqueue(info);
});

// Trace closures to potentially infer argument types.
types.allocatedClosures.forEach((dynamic info) {
void trace(
Iterable<FunctionEntity> elements, ClosureTracerVisitor tracer) {
tracer.run();
if (!tracer.continueAnalyzing) {
elements.forEach((FunctionEntity element) {
inferredDataBuilder.registerMightBePassedToApply(element);
if (debug.VERBOSE) {
print("traced closure $element as ${true} (bail)");
}
Set<FunctionEntity> bailedOutOn = Set<FunctionEntity>();

// Trace closures to potentially infer argument types.
types.allocatedClosures.forEach((dynamic info) {
void trace(
Iterable<FunctionEntity> elements, ClosureTracerVisitor tracer) {
tracer.run();
if (!tracer.continueAnalyzing) {
elements.forEach((FunctionEntity element) {
inferredDataBuilder.registerMightBePassedToApply(element);
if (debug.VERBOSE) {
print("traced closure $element as ${true} (bail)");
}
types.strategy.forEachParameter(element, (Local parameter) {
types
.getInferredTypeOfParameter(parameter)
.giveUp(this, clearInputs: false);
});
});
bailedOutOn.addAll(elements);
return;
}
elements
.where((e) => !bailedOutOn.contains(e))
.forEach((FunctionEntity element) {
types.strategy.forEachParameter(element, (Local parameter) {
types
.getInferredTypeOfParameter(parameter)
.giveUp(this, clearInputs: false);
ParameterTypeInformation info =
types.getInferredTypeOfParameter(parameter);
info.maybeResume();
_workQueue.add(info);
});
if (tracer.tracedType.mightBePassedToFunctionApply) {
inferredDataBuilder.registerMightBePassedToApply(element);
}
if (debug.VERBOSE) {
print("traced closure $element as "
"${inferredDataBuilder.getCurrentlyKnownMightBePassedToApply(element)}");
}
});
bailedOutOn.addAll(elements);
return;
}
elements
.where((e) => !bailedOutOn.contains(e))
.forEach((FunctionEntity element) {
types.strategy.forEachParameter(element, (Local parameter) {
ParameterTypeInformation info =
types.getInferredTypeOfParameter(parameter);
info.maybeResume();
_workQueue.add(info);
});
if (tracer.tracedType.mightBePassedToFunctionApply) {
inferredDataBuilder.registerMightBePassedToApply(element);
}
if (debug.VERBOSE) {
print("traced closure $element as "
"${inferredDataBuilder.getCurrentlyKnownMightBePassedToApply(element)}");
}
});
}

if (info is ClosureTypeInformation) {
Iterable<FunctionEntity> elements = [info.closure];
trace(elements, ClosureTracerVisitor(elements, info, this));
} else if (info is CallSiteTypeInformation) {
final selector = info.selector;
if (info is StaticCallSiteTypeInformation &&
selector != null &&
selector.isCall) {
// This is a constructor call to a class with a call method. So we
// need to trace the call method here.
final calledElement = info.calledElement;
assert(calledElement is ConstructorEntity &&
calledElement.isGenerativeConstructor);
final cls = calledElement.enclosingClass!;
final callMethod = _lookupCallMethod(cls)!;
Iterable<FunctionEntity> elements = [callMethod];
trace(elements, ClosureTracerVisitor(elements, info, this));
} else {
// We only are interested in functions here, as other targets
// of this closure call are not a root to trace but an intermediate
// for some other function.
Iterable<FunctionEntity> elements = List<FunctionEntity>.from(
info.callees.where((e) => e.isFunction));
if (info is ClosureTypeInformation) {
Iterable<FunctionEntity> elements = [info.closure];
trace(elements, ClosureTracerVisitor(elements, info, this));
} else if (info is CallSiteTypeInformation) {
final selector = info.selector;
if (info is StaticCallSiteTypeInformation &&
selector != null &&
selector.isCall) {
// This is a constructor call to a class with a call method. So we
// need to trace the call method here.
final calledElement = info.calledElement;
assert(calledElement is ConstructorEntity &&
calledElement.isGenerativeConstructor);
final cls = calledElement.enclosingClass!;
final callMethod = _lookupCallMethod(cls)!;
Iterable<FunctionEntity> elements = [callMethod];
trace(elements, ClosureTracerVisitor(elements, info, this));
} else {
// We only are interested in functions here, as other targets
// of this closure call are not a root to trace but an intermediate
// for some other function.
Iterable<FunctionEntity> elements = List<FunctionEntity>.from(
info.callees.where((e) => e.isFunction));
trace(elements, ClosureTracerVisitor(elements, info, this));
}
} else if (info is MemberTypeInformation) {
final member = info.member as FunctionEntity;
trace(
[member], StaticTearOffClosureTracerVisitor(member, info, this));
} else if (info is ParameterTypeInformation) {
failedAt(NO_LOCATION_SPANNABLE,
'Unexpected closure allocation info $info');
}
} else if (info is MemberTypeInformation) {
final member = info.member as FunctionEntity;
trace([member], StaticTearOffClosureTracerVisitor(member, info, this));
} else if (info is ParameterTypeInformation) {
failedAt(
NO_LOCATION_SPANNABLE, 'Unexpected closure allocation info $info');
}
});
});

dump?.beforeTracing();
Expand Down Expand Up @@ -1137,6 +1140,7 @@ class _InferrerEngineMetrics extends MetricsBase {
final time = DurationMetric('time');
final analyze = DurationMetric('time.analyze');
final refine1 = DurationMetric('time.refine1');
final trace = DurationMetric('time.trace');
final refine2 = DurationMetric('time.refine2');
final elementsInGraph = CountMetric('count.elementsInGraph');
final allTypesCount = CountMetric('count.allTypes');
Expand All @@ -1148,6 +1152,7 @@ class _InferrerEngineMetrics extends MetricsBase {
secondary = [
analyze,
refine1,
trace,
refine2,
elementsInGraph,
allTypesCount,
Expand Down
11 changes: 5 additions & 6 deletions pkg/compiler/lib/src/inferrer/node_tracer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,12 +473,11 @@ abstract class TracerVisitor implements TypeInformationVisitor {
bailout('Passed to noSuchMethod');
}

Iterable<TypeInformation> inferredTargetTypes =
info.concreteTargets.map((MemberEntity entity) {
return inferrer.types.getInferredTypeOfMember(entity);
});
if (inferredTargetTypes.any((user) => user == currentUser)) {
addNewEscapeInformation(info);
final user = currentUser;
if (user is MemberTypeInformation) {
if (info.concreteTargets.contains(user.member)) {
addNewEscapeInformation(info);
}
}
}

Expand Down
15 changes: 9 additions & 6 deletions pkg/compiler/lib/src/inferrer_experimental/closure_tracer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,6 @@ class ClosureTracerVisitor extends TracerVisitor {
}
}

bool _checkIfCurrentUser(MemberEntity element) =>
inferrer.types.getInferredTypeOfMember(element) == currentUser;

bool _checkIfFunctionApply(MemberEntity element) {
return inferrer.closedWorld.commonElements.isFunctionApplyMethod(element);
}
Expand All @@ -118,17 +115,23 @@ class ClosureTracerVisitor extends TracerVisitor {
visitDynamicCallSiteTypeInformation(DynamicCallSiteTypeInformation info) {
super.visitDynamicCallSiteTypeInformation(info);
final selector = info.selector!;
final user = currentUser;
if (selector.isCall) {
if (info.arguments!.contains(currentUser)) {
if (info.arguments!.contains(user)) {
if (info.hasClosureCallTargets ||
info.concreteTargets.any((element) => !element.isFunction)) {
bailout('Passed to a closure');
}
if (info.concreteTargets.any(_checkIfFunctionApply)) {
_tagAsFunctionApplyTarget("dynamic call");
}
} else if (info.concreteTargets.any(_checkIfCurrentUser)) {
_registerCallForLaterAnalysis(info);
} else {
if (user is MemberTypeInformation) {
final currentUserMember = user.member;
if (info.concreteTargets.contains(currentUserMember)) {
_registerCallForLaterAnalysis(info);
}
}
}
} else if (selector.isGetter && selector.memberName == Names.call) {
// We are potentially tearing off ourself here
Expand Down
Loading

0 comments on commit 5829f93

Please sign in to comment.